Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
deps: backport 2bcbe2f from V8 upstream#7814
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ofrobots commented Jul 21, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
bnoordhuis commented Jul 21, 2016
LGTM but out of curiosity, is there a reason you opted for line buffering instead of no buffering? It seems the output is line oriented anyways so libc's newline scanning just adds unnecessary, if minimal, overhead. |
ofrobots commented Jul 21, 2016
No strong reason other than to avoid any file watchers from observing partially complete lines. The output is indeed line oriented today; but the code may change in the future. The new line scanning is going to be negligible cost. |
bab39d4 to bf3d4b2Compareofrobots commented Jul 26, 2016
Rebased, added a V8 patch-level bump. Relaunched CI: https://ci.nodejs.org/job/node-test-pull-request/3413/. |
ofrobots commented Jul 26, 2016
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/244/. I think the Power BE machine has infrastructure issues. The rest are green. |
ofrobots commented Jul 29, 2016
@nodejs/release: this is ready for landing onto |
evanlucas commented Jul 29, 2016
works for me :] |
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015 [email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{nodejs#36788} PR-URL: nodejs#7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
bf3d4b2 to e82e804Compareofrobots commented Jul 29, 2016
Thanks. Landed on |
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015 [email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{#36788} PR-URL: nodejs/node#7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015[email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{#36788} PR-URL: #7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015[email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{#36788} PR-URL: #7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015[email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{#36788} PR-URL: #7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Excessive buffering of perf map files in V8 could cause profiles to be missing symbols at times. Original commit message: switch perf and ll_prof loggers to line buffering BUG=v8:5015 [email protected],[email protected] Review-Url: https://codereview.chromium.org/2041243002 Cr-Commit-Position: refs/heads/master@{#36788} PR-URL: nodejs/node#7814 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
Description of change
Note that this is not needed on
masteras V8 5.1 already contains this fix.R=@nodejs/v8
EDIT: CI: https://ci.nodejs.org/job/node-test-pull-request/3362/