Repository navigation
async stack tracking should not be enabled by default #16180
Description
#13870 adds support for async stack tracking whenever the inspector agent is enabled. Async stack traces are awesome for debugging, but doing this by automatically when the inspector agent is active has a lot of problems:
Some problematic scenarios I can think of:
- User wants to attach DevTools in order to profile their app. Enabling async stack traces add overhead that was not present before the profiler was attached. The profile would not be representative of the original program.
- User wants to take a heap profile and uses the inspector module to take the heap profile. Async stack tracking gets enabled as a side-effect – something that the user did not want (the user is not 'single step debugging').
- User wants to use the inspector module to build a passive, in-production debugger. User is not interested in the overhead of async stack tracking.
Async stack tracking should have been an opt in. Whether or not the inspector agent is enabled conflates too many use-cases.
Apologies that I didn't catch this issue earlier.
/cc @nodejs/v8-inspector @bajtos @nodejs/diagnostics
Activity
bajtos commentedon Oct 13, 2017
@ofrobots good points. IIRC, DevTools have a checkbox to enable/disable async stack traces in the debugger. Under the hood, DevTools send
Debugger.setAsyncCallStackDepthwith a non-zero value (I think it's always10) to enable async stack traces in V8 and zero to disable them. When the async call stack depth is 0, the V8AsyncTask*API calls are effectively no-ops, at least it was this way the last time I checked V8 sources.So I think the only overhead of enabling async stack traces lies in the overhead of Node.js's async_hooks machinery, which is still likely to change the performance profile of the inspected app as you pointed out.
I think it would be best if Node.js could intercept
Debugger.setAsyncCallStackDepthcommands send by inspector clients (e.g. DevTools front-end) and enable/disable the async_hook depending on whether the front-end requested async stack traces or not.Thoughts?
AndreasMadsen commentedon Oct 13, 2017
That would be amazing. But we need to read the
asyncCallStackDepthon startup otherwise we won't have the full context. Is that possible?TimothyGu commentedon Oct 13, 2017
Interception is totally possible. Reading it at startup isn't as big of a concern, since right now we allow enabling inspection during runtime and tasks started before starting inspection are not tracked anyway.
refack commentedon Oct 14, 2017
As I mentioned in IRC I would consider this
semver-minor./cc @jkrems, @roblourens, @billti, @prigara, @ulitink, @segrey
prigara commentedon Oct 16, 2017
@refack Thanks for the heads up.
Our current plan is to keep async stack traces enabled by default. Right now WebStorm doesn’t support heap profiling with the new protocol, so it shouldn’t be affected by the issues mentioned in the first comment.
Are there any other known issues related to having async stack tracking enabled?
ofrobots commentedon Oct 16, 2017
@prigara The OP mentions a few cases other than heap profiler (e.g. CPU profiling, passive debugging).
IMO, it is not an option to keep by-default stack tracking. I would like to get this fixed before we reach LTS for Node 8.x. I'm looking into intercepting
Debugger.setAsyncCallStackDepthas a mechanism to enable async tracking. Perhaps we can add an additional command line flag that allows folks to enable async tracking on process startup.ofrobots commentedon Oct 16, 2017
Interception of the
Debugger.setAsyncCallStackDepthmessage is somewhat complicated as it has to be implemented in C++.With
--inspect-brk, the message arrives on the wire way too early (before JavaScript has bootstrapped). This means that the interception function cannot be JavaScript. Having to parse JSON on the C++ side in the inspector Agent is starting to look very clunky.refack commentedon Oct 16, 2017
@prigara with the hooks enabled, the debugger steps into the hooks scripts for all of the invocations (before and after async operations), and when stepping through the hook invocation script makes the async stack suddenly appear
@ofrobots I might be wrong, but I think that if the inspector Agent will send
Debugger.setAsyncCallStackDepthonly after the client has attached, it might be good enough.ofrobots commentedon Oct 17, 2017
PR: #16260
inspector: track async stacks when necessary
8 remaining items
inspector: track async stacks when necessary
inspector: track async stacks when necessary
inspector: track async stacks when necessary
inspector: track async stacks when necessary
inspector: track async stacks when necessary
Zorono commentedon Oct 29, 2019
excuse me, but i am facing the same error recently when i was trying to inspect/debug my NodeJS Code...
what about that
Fix(#16308) ?? (really my mind is blowing...)Node.JS version:
v10.16.3NPM version:
v6.12.0Device Operating System:
Windows 7 Pro(platform:x32)i tried inspecting using
--inspectarg andnpm i node-inspectpackage and Visual Studio 2019 which i think is using--inspect(if iamn't wrong about the method it is doing it`) but all of them are showing the same warning...