This repository was archived by the owner on Apr 22, 2023. It is now read-only.
v8: don't busy loop in cpu profiler thread#8789
Closed
bnoordhuis wants to merge 1 commit intonodejs:v0.10from
Closed
v8: don't busy loop in cpu profiler thread#8789bnoordhuis wants to merge 1 commit intonodejs:v0.10from
bnoordhuis wants to merge 1 commit intonodejs:v0.10from
Conversation
Reduce the overhead of the CPU profiler by replacing sched_yield() with nanosleep() in V8's tick event processor thread. The former only yields the CPU when there is another process scheduled on the same CPU. Before this commit, the thread would effectively busy loop and consume 100% CPU time. By forcing a one nanosecond sleep period rounded up to the task scheduler's granularity (about 50 us on Linux), CPU usage for the processor thread now hovers around 10-20% for a busy application. Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
|
@bnoordhuis Is this a change that will be upstream'd? Either way, nice change. After you ping back I'll land this. |
Member
Author
|
I don't think there is anything I can upstream. 3.14 is dead upstream and newer V8 versions don't seem to have this issue (or at least, I haven't seen it myself and none of our customers have complained about it so far.) |
|
Wow, that's quite a substantial improvement. Great job! |
Member
Author
|
@trevnorris Ping. |
trevnorris
pushed a commit
that referenced
this pull request
Jan 13, 2015
Reduce the overhead of the CPU profiler by replacing sched_yield() with nanosleep() in V8's tick event processor thread. The former only yields the CPU when there is another process scheduled on the same CPU. Before this commit, the thread would effectively busy loop and consume 100% CPU time. By forcing a one nanosecond sleep period rounded up to the task scheduler's granularity (about 50 us on Linux), CPU usage for the processor thread now hovers around 10-20% for a busy application. PR-URL: #8789 Ref: strongloop/strong-agent#3 Reviewed-by: Trevor Norris <trev.norris@gmail.com>
|
@bnoordhuis Thanks for the ping. Landed in 6ebd85e. |
tunniclm
added a commit
to tunniclm/node-v0.x-archive
that referenced
this pull request
May 8, 2015
Backport 6964a9e0685fa186d9d9b7907be17505e839db1a from upstream v8. Original commit message: Make CPU profiler do not hog 100% of CPU. Tick event processor should not stay in a tight loop when there's nothing to do. It can go sleep until next sample event. LOG=N BUG=v8:3967 Committed: https://crrev.com/6964a9e0685fa186d9d9b7907be17505e839db1a Cr-Commit-Position: refs/heads/master@{#28211} Fixes nodejs#25137 Related: nodejs#9439, nodejs#8789
misterdjules
pushed a commit
to misterdjules/node
that referenced
this pull request
May 12, 2015
Backport 6964a9e0685fa186d9d9b7907be17505e839db1a from upstream v8. Original commit message: Make CPU profiler do not hog 100% of CPU. Tick event processor should not stay in a tight loop when there's nothing to do. It can go sleep until next sample event. LOG=N BUG=v8:3967 Committed: https://crrev.com/6964a9e0685fa186d9d9b7907be17505e839db1a Cr-Commit-Position: refs/heads/master@{#28211} Fixes nodejs#25137 Related: nodejs#9439, nodejs#8789 PR: nodejs#25268 PR-URL: nodejs#25268 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduce the overhead of the CPU profiler by replacing sched_yield() with
nanosleep() in V8's tick event processor thread. The former only yields
the CPU when there is another process scheduled on the same CPU.
Before this commit, the thread would effectively busy loop and consume
100% CPU time. By forcing a one nanosecond sleep period rounded up to
the task scheduler's granularity (about 50 us on Linux), CPU usage for
the processor thread now hovers around 10-20% for a busy application.
Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
R=@trevnorris?