Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf(ext/console): avoid wrapConsole when not inspecting #15931

Merged
merged 5 commits into from
Sep 17, 2022
Merged

perf(ext/console): avoid wrapConsole when not inspecting #15931

merged 5 commits into from
Sep 17, 2022

Conversation

littledivy
Copy link
Member

@littledivy littledivy commented Sep 17, 2022 •
edited
Loading

Fixes #11932

3x improvement logging strings.

time deno run for.js > /dev/null

________________________________________________________
Executed in  308.64 millis    fish           external
   usr time  297.68 millis    0.24 millis  297.43 millis
   sys time   37.82 millis    1.63 millis   36.19 millis

time target/release/deno run for.js > /dev/null

________________________________________________________
Executed in  109.55 millis    fish           external
   usr time  104.32 millis  130.00 micros  104.19 millis
   sys time   33.39 millis  840.00 micros   32.55 millis

@ghost
Copy link

ghost commented Sep 17, 2022

Can you confirm that this doesn't degrade performance for stdout writing?
That would be unfortunate, but I don't know which case is more common.

littledivy
littledivy commented Sep 17, 2022
View reviewed changes
cli/tests/testdata/event_listener_error_immediate_exit.ts.out
@@ -1,4 +1,5 @@
1
queueMicrotask
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nayeemrmn @lucacasonato is this test supposed to be always deterministic? queueMicrotask is called now maybe because we don't enter v8 callconsole?

@littledivy
Copy link
Member Author

Can you confirm that this doesn't degrade performance for stdout writing?

@phosra you mean Deno.stdin.writeSync API? This doesn't touch that, there is another PR to convert those ops to fast calls #15863

@ghost
Copy link

ghost commented Sep 17, 2022

@phosra you mean Deno.stdin.writeSync API? This doesn't touch that, there is another PR to convert those ops to fast calls #15863

Nvm, mb, had the wrong idea of what the PR did!

(Keep up the great work!)

@littledivy littledivy merged commit 6154188 into denoland:main Sep 17, 2022
dsherret pushed a commit that referenced this pull request Sep 22, 2022
@littledivy @dsherret
perf(ext/console): avoid wrapConsole when not inspecting (#15931)
bfbc5dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

console.log is slow
2 participants
@littledivy @bartlomieju