Register v8 debugger as async event delegate when tracing [v8/v8 : main]
Andres Olivares (Gerrit)
Andres Olivares voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andres Olivares
| Commit-Queue | +1 |
1 comment
Related details
- Benedikt Meurer
- Simon Zünd
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
called when tracing as the optimized path prevents calling into this
when no async event delegate is registered [1]. The implementation ofThis is very much on purpose. Transitioning from JS to C++ and back into JS is very costly. Installing an async event delegate means that a lot of code paths will revert to the slow-path, and in the worst case cause deopts. Adding this additional async/promise instrumentation will heavily skew async/promise-heavy code vs plain sync code in performance profiles.
Did you run some benchmarks to measure what the overhead is, and if the performance hit is acceptable?
Related details
- Andres Olivares
- Benedikt Meurer
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andres Olivares (Gerrit)
Andres Olivares added 1 comment![]( "Open in Gerrit")
called when tracing as the optimized path prevents calling into this
when no async event delegate is registered [1]. The implementation ofThis is very much on purpose. Transitioning from JS to C++ and back into JS is very costly. Installing an async event delegate means that a lot of code paths will revert to the slow-path, and in the worst case cause deopts. Adding this additional async/promise instrumentation will heavily skew async/promise-heavy code vs plain sync code in performance profiles.
Did you run some benchmarks to measure what the overhead is, and if the performance hit is acceptable?
I was assuming the callpaths enabled by this would be the ones that start from the call to trace (given the still valid early return in the async task instrumentation methods), but it seems this is not the case given that the call into C++ wouldn't happen otherwise? Do you have a benchmark you can recommend we test the impact of this change with?
Related details
- Benedikt Meurer
- Simon Zünd
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andres Olivares (Gerrit)
Andres Olivares abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Benedikt Meurer (Gerrit)
Benedikt Meurer added 1 comment![]( "Open in Gerrit")
v8::debug::SetAsyncEventDelegate(Are you sure that this method will (re)run when tracing is enabled?
Related details
- Andres Olivares
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Simon Zünd (Gerrit)
Simon Zünd added 1 comment![]( "Open in Gerrit")
v8::debug::SetAsyncEventDelegate(Are you sure that this method will (re)run when tracing is enabled?
I think it's more of a side-effect that DevTools explicitly disables the debugger before starting the profiler.
Related details
- Andres Olivares
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andres Olivares (Gerrit)
Andres Olivares added 1 comment![]( "Open in Gerrit")
v8::debug::SetAsyncEventDelegate(Simon ZündAre you sure that this method will (re)run when tracing is enabled?
I think it's more of a side-effect that DevTools explicitly disables the debugger before starting the profiler.
I've tested this locally and it works, likely through the call path Simon mentions. But in any case, I've abandoned this change until we have a better idea of its usefuleness and impact in performance to determine if it's worth landing. Thanks!
Related details
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |