[v9.x backport] deps: cherry-pick 15c0c3a8ba and 43e2fb1c3d from upstream V8#19333
Uh oh!
There was an error while loading. Please reload this page.
Conversation
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
nodejs-github-bot added build 
Contributor
MylesBorins commented Mar 14, 2018
Contributor
MylesBorins commented Mar 14, 2018
MemberAuthor
Flarna commented Mar 14, 2018
Splitting this PR into two PRs doesn't make sense as the second commit fixes a bug introduced with the first. We should never end up in having just one of them. |
Contributor
MylesBorins commented Mar 14, 2018
@Flarna that makes sense /cc @nodejs/platform-aix re: failure |
Member
richardlau commented Mar 14, 2018
test-performance was recently moved on master from parallel to sequential (#19228) as it was flaky. |
Contributor
ofrobots left a comment
ofrobots left a comment There was a problem hiding this comment.
LGTM w/ nit: the commit message abstract is too long. I would recommend something like:
deps: v8: cherry-pick fixes for v8:7535
These changes avoid a busy wait loop in V8 CPU Profiler thread for windows (except for short intervals). It would be good if this is also backported to Node.js v9 and LTS releases as this busy loop effectively disallows the use of cpu-profiler in windows production setups. Original commit message 15c0c3a8ba: ``` [profiler] use Sleep() on windows for long profile intervals. See nodejs/diagnostics#170[email protected] Change-Id: Iecc3bb27707b0d2afbb23fd9823d5cd4d725be6e Reviewed-on: https://chromium-review.googlesource.com/931102 Reviewed-by: Franziska Hinkelmann <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#51466} ``` Original commit message 43e2fb1c3d: ``` [profiler] fix sleeping on windows for long intervals. [email protected] Change-Id: I5717db794fc797e7c3b0b8f122ddb6dc0702a99e Reviewed-on: https://chromium-review.googlesource.com/941126 Reviewed-by: Franziska Hinkelmann <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#51755} ``` Refs: nodejs/diagnostics#170 Refs: #19200 Refs: v8/v8@15c0c3a Refs: v8/v8@43e2fb1
MemberAuthor
Flarna commented Mar 16, 2018
I have updated commit messages as suggested by @ofrobots |
MylesBorinsforce-pushed the v9.x-staging branch 4 times, most recently from d457b9d to 03c321aCompareMylesBorins pushed a commit that referenced this pull request Mar 20, 2018
These changes avoid a busy wait loop in V8 CPU Profiler thread for windows (except for short intervals). It would be good if this is also backported to Node.js v9 and LTS releases as this busy loop effectively disallows the use of cpu-profiler in windows production setups. Original commit message 15c0c3a8ba: ``` [profiler] use Sleep() on windows for long profile intervals. See nodejs/diagnostics#170[email protected] Change-Id: Iecc3bb27707b0d2afbb23fd9823d5cd4d725be6e Reviewed-on: https://chromium-review.googlesource.com/931102 Reviewed-by: Franziska Hinkelmann <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#51466} ``` Original commit message 43e2fb1c3d: ``` [profiler] fix sleeping on windows for long intervals. [email protected] Change-Id: I5717db794fc797e7c3b0b8f122ddb6dc0702a99e Reviewed-on: https://chromium-review.googlesource.com/941126 Reviewed-by: Franziska Hinkelmann <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#51755} ``` PR-URL: #19333 Refs: nodejs/diagnostics#170 Refs: #19200 Refs: v8/v8@15c0c3a Refs: v8/v8@43e2fb1 Reviewed-By: Myles Borins <[email protected]>
Contributor
MylesBorins commented Mar 20, 2018
landed in 5966b8c |
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
5 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.
These changes avoid a busy wait loop in V8 CPU Profiler thread for windows (except for short intervals).
It would be good if this is also backported to Node.js v9 and LTS releases as this busy loop effectively disallows the use of CpuProfiler in Windows production setups.
Original commit message 15c0c3a8ba:
Original commit message 43e2fb1c3d:
Refs: nodejs/diagnostics#170
Refs: #19200
Refs: v8/v8@15c0c3a
Refs: v8/v8@43e2fb1
Checklist