Skip to content

[v8.x backport] inspector: Track async stack only when necessary - #16590

Closed
ofrobots wants to merge 2 commits into
nodejs:v8.x-stagingfrom
ofrobots:backport-16308
Closed

[v8.x backport] inspector: Track async stack only when necessary#16590
ofrobots wants to merge 2 commits into
nodejs:v8.x-stagingfrom
ofrobots:backport-16308

Conversation

@ofrobots

@ofrobotsofrobots commented Oct 29, 2017

Copy link
Copy Markdown
Contributor

This is the 8.x backport of #16308 as the version of V8 in 8.x is slightly older than master and some changes were necessary.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps:v8, inspector

R= @nodejs/v8 @nodejs/v8-inspector

CI: https://ci.nodejs.org/job/node-test-pull-request/11104/
V8-CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/1025/

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Oct 29, 2017
@ofrobotsofrobots added inspector Issues and PRs related to the V8 inspector protocol dont-land-on-v4.x labels Oct 29, 2017
@gibfahn

Copy link
Copy Markdown
Member

@ofrobots looks like this conflicts with something I just landed, would you mind taking a look?

diff:

<<<<<<< HEAD
// Enable tracking of async stack traces
if (!enable_async_hook_function_.IsEmpty()) {
Local<Function> enable_fn = enable_async_hook_function_.Get(isolate);
auto result = enable_fn->Call(context, Undefined(isolate), 0, nullptr);
if (result.IsEmpty()) {
FatalError(
"node::InspectorAgent::StartIoThread",
"Cannot enable Inspector's AsyncHook, please report this.");
}
}
||||||| parent of 2069010344... inspector: track async stacks when necessary
// Enable tracking of async stack traces
if (!enable_async_hook_function_.IsEmpty()) {
Local<Function> enable_fn = enable_async_hook_function_.Get(isolate);
auto context = parent_env_->context();
auto result = enable_fn->Call(context, Undefined(isolate), 0, nullptr);
if (result.IsEmpty()) {
FatalError(
"node::InspectorAgent::StartIoThread",
"Cannot enable Inspector's AsyncHook, please report this.");
}
}
=======
>>>>>>> 2069010344... inspector: track async stacks when necessary

@gibfahn
gibfahnforce-pushed the v8.x-staging branch 7 times, most recently from 97c2301 to ab0d7a6CompareOctober 31, 2017 00:16
Original commit message:
[inspector] added V8InspectorClient::maxAsyncCallStackDepthChanged
R=dgozman@chromium.org
Bug: none
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I0fa10978266feb3c3907ce1f3386ae7a34a33582
Reviewed-on: https://chromium-review.googlesource.com/726490
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#48705}
PR-URL: nodejs#16308
Refs: v8/v8@b1cd96e
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
With this change, we do async stack tracking only when explicitly
requested by the inspector client. This avoids unnecessary overhead
for clients that might not be interested in async stack traces.
PR-URL: nodejs#16308Fixes: nodejs#16180
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@ofrobots

Copy link
Copy Markdown
ContributorAuthor

@gibfahn Conflicts resolved.

MylesBorins pushed a commit that referenced this pull request Oct 31, 2017
Original commit message:
[inspector] added V8InspectorClient::maxAsyncCallStackDepthChanged
R=dgozman@chromium.org
Bug: none
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I0fa10978266feb3c3907ce1f3386ae7a34a33582
Reviewed-on: https://chromium-review.googlesource.com/726490
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48705}
Backport-PR-URL: #16590
PR-URL: #16308
Refs: v8/v8@b1cd96e
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 31, 2017
With this change, we do async stack tracking only when explicitly
requested by the inspector client. This avoids unnecessary overhead
for clients that might not be interested in async stack traces.
Backport-PR-URL: #16590
PR-URL: #16308Fixes: #16180
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

landed in ab0d7a6...2ea25ad

@ofrobots
ofrobots deleted the backport-16308 branch October 31, 2017 03:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inspectorIssues and PRs related to the V8 inspector protocollib / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ofrobots@gibfahn@MylesBorins@nodejs-github-bot