Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Backport async hooks to v8.x #18179

Closed
wants to merge 22 commits into from
Closed

Backport async hooks to v8.x #18179

wants to merge 22 commits into from

Conversation

AndreasMadsen
Copy link
Member

@AndreasMadsen AndreasMadsen commented Jan 16, 2018 •
edited
Loading

Please merge this as fast as possible, I don't want to do it again because some commit got merged into v8.x-staging.

This contains a backport of the follow commits:

The list was generated with

branch-diff v8.x master --exclude-label=semver-major,dont-land-on-v8.x --filter-release --format=simple | tee v8.x-branch-diff
cat v8.x-branch-diff | grep -e async_hooks -e tracing -e trace_events -e 17071 -e 16565 -e async_wrap -e 17117 -e 17064 -e 17757
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

async_hooks, trace_events, src, async_wrap, timers, http, test

yhwang and others added 22 commits January 16, 2018 13:00
@yhwang @AndreasMadsen
src: explicitly register built-in modules
7a5e327 
Previously, built-in modules are registered before main() via
__attribute__((constructor)) mechanism in GCC and similiar
mechanism in MSVC. This causes some issues when node is built as
static library. Calling module registration function for built-in
modules in node::Init() helps to avoid the issues.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>
PR-URL: #16565
Refs: #14986 (comment)
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@fhinkel @AndreasMadsen
src: use unique pointer for tracing_agent
6e98aa9 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

PR-URL: #17012
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@AndreasMadsen
async_hooks: add trace events to async_hooks
2bffd70 
This will allow trace event to record timing information for all
asynchronous operations that are observed by async_hooks.

PR-URL: #15538
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Sebmaster @AndreasMadsen
async_hooks: add destroy event for gced AsyncResources
07e7f2e 
In cases where libraries create AsyncResources which may be emitting
more events depending on usage, the only way to ensure that destroy is
called properly is by calling it when the resource gets garbage
collected.

Fixes: #16153
PR-URL: #16998
Fixes: #16153
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis @AndreasMadsen
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
1a66f91 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

PR-URL: #17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@danbev @AndreasMadsen
src: rename async-wrap -> async_wrap
b7e3109 
This commit renames async-wrap to async_wrap for consitency with other
c++ source files.

PR-URL: #17022
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@addaleax @AndreasMadsen
timers: cross JS/C++ border less frequently
8c83406 
This removes the `process._needImmediateCallback` property
and its semantics of having a 1/0 switch that tells C++ whether
immediates are currently scheduled.

Instead, a counter keeping track of all immediates is created,
that can be increased on `setImmediate()` or decreased when an
immediate is run or cleared.

This is faster, because rather than reading/writing a C++ getter,
this operation can be performed as a direct memory read/write via
a typed array. The only C++ call that is left to make is
activating the native handles upon creation of the first
`Immediate` after the queue is empty.

One other (good!) side-effect is that `immediate._destroyed` now
reliably tells whether an `immediate` is still scheduled to run or not.

Also, as a nice extra, this should make it easier to implement
an internal variant of `setImmediate` for C++ that piggybacks
off the same mechanism, which should be useful at least for
async hooks and HTTP/2.

Benchmark results:

    $ ./node benchmark/compare.js --new ./node --old ./node-master-1b093cb93df0 --runs 10 --filter immediate timers | Rscript benchmark/compare.R
    [00:08:53|% 100| 4/4 files | 20/20 runs | 1/1 configs]: Done
                                                         improvement confidence      p.value
     timers/immediate.js type="breadth" thousands=2000      25.61 %         ** 1.432301e-03
     timers/immediate.js type="breadth1" thousands=2000      7.66 %            1.320233e-01
     timers/immediate.js type="breadth4" thousands=2000      4.61 %            5.669053e-01
     timers/immediate.js type="clear" thousands=2000       311.40 %        *** 3.896291e-07
     timers/immediate.js type="depth" thousands=2000        17.54 %         ** 9.755389e-03
     timers/immediate.js type="depth1" thousands=2000       17.09 %        *** 7.176229e-04
     timers/set-immediate-breadth-args.js millions=5        10.63 %          * 4.250034e-02
     timers/set-immediate-breadth.js millions=10            20.62 %        *** 9.150439e-07
     timers/set-immediate-depth-args.js millions=10         17.97 %        *** 6.819135e-10

PR-URL: #17064
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@AndreasMadsen
async_hooks: deprecate undocumented API
d66d481 
PR-URL: #16972
Refs: #14328
Refs: #15572
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax @AndreasMadsen
src: introduce internal C++ SetImmediate() mechanism
c904ce1 
PR-URL: #17117
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @AndreasMadsen
src: remove async_hooks destroy timer handle
590cf4a 
PR-URL: #17117
Reviewed-By: James M Snell <jasnell@gmail.com>
@AndreasMadsen
trace_events: add executionAsyncId to init events
d32d180 
async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

PR-URL: #17196
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@AndreasMadsen
async_wrap: add provider types for net server
44cbf56 
Adds `TCPSERVERWRAP` and `PIPESERVERWRAP` as provider types. This
makes it possible to distinguish servers from connections.

PR-URL: #17157
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@AndreasMadsen
async_hooks: rename initTriggerId
1044e76 
rename initTriggerId to defaultTriggerAsyncId such it matches the rest
of our naming.

PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@AndreasMadsen
async_hooks: separate missing from default context
a04d5ee 
When context is missing the executionAsyncId will be zero. For the
default triggerAsyncId the zero value was used to default to the
executionAsyncId. While this was not technically wrong because the
functions are different themself, it poorly separated the two concepts.

PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@AndreasMadsen
async_hooks: use scope for defaultTriggerAsyncId
44f4c73 
Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.

PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @AndreasMadsen
src: remove unused async hooks methods
7310f66 
PR-URL: #17757
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: vdeturckheim <vlad2t@hotmail.com>
@maclover7 @AndreasMadsen
async_hooks: use CHECK instead of throwing error
4e656fd 
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

PR-URL: #17832
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@addaleax @AndreasMadsen
async_hooks: use typed array stack as fast path
067bd2a 
- Communicate the current async stack length through a
  typed array field rather than a native binding method
- Add a new fixed-size `async_ids_fast_stack` typed array
  that contains the async ID stack up to a fixed limit.
  This increases performance noticeably, since most of the time
  the async ID stack will not be more than a handful of
  levels deep.
- Make the JS `pushAsyncIds()` and `popAsyncIds()` functions
  do the same thing as the native ones if the fast path
  is applicable.

Benchmarks:

    $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter next-tick process | Rscript benchmark/compare.R
    [00:03:25|% 100| 6/6 files | 20/20 runs | 1/1 configs]: Done
                                                   improvement confidence      p.value
     process/next-tick-breadth-args.js millions=4     19.72 %        *** 3.013913e-06
     process/next-tick-breadth.js millions=4          27.33 %        *** 5.847983e-11
     process/next-tick-depth-args.js millions=12      40.08 %        *** 1.237127e-13
     process/next-tick-depth.js millions=12           77.27 %        *** 1.413290e-11
     process/next-tick-exec-args.js millions=5        13.58 %        *** 1.245180e-07
     process/next-tick-exec.js millions=5             16.80 %        *** 2.961386e-07

PR-URL: #17780
Reviewed-By: James M Snell <jasnell@gmail.com>
@AndreasMadsen
trace_events: stop tracing agent in process.exit()
af928ef 
PR-URL: #18005
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@apapirovski @AndreasMadsen
async_hooks: update defaultTriggerAsyncIdScope for perf
1efac0e 
The existing version of defaultTriggerAsyncIdScope creates an Array
for the callback's arguments which is highly inefficient. Instead,
use rest syntax and allow V8 to do that work for us. This yields
roughly 2x performance for this particular function.

PR-URL: #18004
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@AndreasMadsen
async_hooks,http: set HTTPParser trigger to socket
3ba594f 
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.

PR-URL: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@AndreasMadsen
async_hooks,test: only use IPv6 in http test
8937440 
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

PR-URL: #18143
Fixes: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Jan 16, 2018
@AndreasMadsen AndreasMadsen mentioned this pull request Jan 16, 2018
Closed
@AndreasMadsen
Copy link
Member Author

/cc @MylesBorins @gibfahn @evanlucas @jasnell

jasnell
jasnell approved these changes Jan 16, 2018
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber stamp LGTM

@gibfahn gibfahn self-assigned this Jan 16, 2018
This was referenced Jan 16, 2018
Closed
Closed
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@addaleax @gibfahn
async_hooks: use typed array stack as fast path
7b9d29b 
- Communicate the current async stack length through a
  typed array field rather than a native binding method
- Add a new fixed-size `async_ids_fast_stack` typed array
  that contains the async ID stack up to a fixed limit.
  This increases performance noticeably, since most of the time
  the async ID stack will not be more than a handful of
  levels deep.
- Make the JS `pushAsyncIds()` and `popAsyncIds()` functions
  do the same thing as the native ones if the fast path
  is applicable.

Benchmarks:

    $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter next-tick process | Rscript benchmark/compare.R
    [00:03:25|% 100| 6/6 files | 20/20 runs | 1/1 configs]: Done
                                                   improvement confidence      p.value
     process/next-tick-breadth-args.js millions=4     19.72 %        *** 3.013913e-06
     process/next-tick-breadth.js millions=4          27.33 %        *** 5.847983e-11
     process/next-tick-depth-args.js millions=12      40.08 %        *** 1.237127e-13
     process/next-tick-depth.js millions=12           77.27 %        *** 1.413290e-11
     process/next-tick-exec-args.js millions=5        13.58 %        *** 1.245180e-07
     process/next-tick-exec.js millions=5             16.80 %        *** 2.961386e-07

PR-URL: nodejs#17780
Backport-PR-URL: nodejs#18179
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@AndreasMadsen @gibfahn
trace_events: stop tracing agent in process.exit()
4087dc8 
PR-URL: nodejs#18005
Backport-PR-URL: nodejs#18179
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@apapirovski @gibfahn
async_hooks: update defaultTriggerAsyncIdScope for perf
a8b7e43 
The existing version of defaultTriggerAsyncIdScope creates an Array
for the callback's arguments which is highly inefficient. Instead,
use rest syntax and allow V8 to do that work for us. This yields
roughly 2x performance for this particular function.

PR-URL: nodejs#18004
Backport-PR-URL: nodejs#18179
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@AndreasMadsen @gibfahn
async_hooks,http: set HTTPParser trigger to socket
5dba883 
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.

PR-URL: nodejs#18003
Backport-PR-URL: nodejs#18179
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@AndreasMadsen @gibfahn
async_hooks,test: only use IPv6 in http test
cc7e20e 
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

PR-URL: nodejs#18143
Backport-PR-URL: nodejs#18179
Fixes: nodejs#18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@yhwang @MylesBorins
src: explicitly register built-in modules
e71beba 
Previously, built-in modules are registered before main() via
__attribute__((constructor)) mechanism in GCC and similiar
mechanism in MSVC. This causes some issues when node is built as
static library. Calling module registration function for built-in
modules in node::Init() helps to avoid the issues.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>
Backport-PR-URL: #18179
PR-URL: #16565
Refs: #14986 (comment)
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@fhinkel @MylesBorins
src: use unique pointer for tracing_agent
ace2c2f 
Use std::unique_ptr instead of raw pointers for the
tracing_agent_ in node.cc. This makes ownership clearer and we
don't risk a memory leak.

Backport-PR-URL: #18179
PR-URL: #17012
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks: add trace events to async_hooks
331b175 
This will allow trace event to record timing information for all
asynchronous operations that are observed by async_hooks.

Backport-PR-URL: #18179
PR-URL: #15538
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@Sebmaster @MylesBorins
async_hooks: add destroy event for gced AsyncResources
36dbd11 
In cases where libraries create AsyncResources which may be emitting
more events depending on usage, the only way to ensure that destroy is
called properly is by calling it when the resource gets garbage
collected.

Backport-PR-URL: #18179
Fixes: #16153
PR-URL: #16998
Fixes: #16153
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@bnoordhuis @MylesBorins
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
aa63e02 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

Backport-PR-URL: #18179
PR-URL: #17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@danbev @MylesBorins
src: rename async-wrap -> async_wrap
f276cd9 
This commit renames async-wrap to async_wrap for consitency with other
c++ source files.

Backport-PR-URL: #18179
PR-URL: #17022
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@addaleax @MylesBorins
timers: cross JS/C++ border less frequently
b5bc3f8 
This removes the `process._needImmediateCallback` property
and its semantics of having a 1/0 switch that tells C++ whether
immediates are currently scheduled.

Instead, a counter keeping track of all immediates is created,
that can be increased on `setImmediate()` or decreased when an
immediate is run or cleared.

This is faster, because rather than reading/writing a C++ getter,
this operation can be performed as a direct memory read/write via
a typed array. The only C++ call that is left to make is
activating the native handles upon creation of the first
`Immediate` after the queue is empty.

One other (good!) side-effect is that `immediate._destroyed` now
reliably tells whether an `immediate` is still scheduled to run or not.

Also, as a nice extra, this should make it easier to implement
an internal variant of `setImmediate` for C++ that piggybacks
off the same mechanism, which should be useful at least for
async hooks and HTTP/2.

Benchmark results:

    $ ./node benchmark/compare.js --new ./node --old ./node-master-1b093cb93df0 --runs 10 --filter immediate timers | Rscript benchmark/compare.R
    [00:08:53|% 100| 4/4 files | 20/20 runs | 1/1 configs]: Done
                                                         improvement confidence      p.value
     timers/immediate.js type="breadth" thousands=2000      25.61 %         ** 1.432301e-03
     timers/immediate.js type="breadth1" thousands=2000      7.66 %            1.320233e-01
     timers/immediate.js type="breadth4" thousands=2000      4.61 %            5.669053e-01
     timers/immediate.js type="clear" thousands=2000       311.40 %        *** 3.896291e-07
     timers/immediate.js type="depth" thousands=2000        17.54 %         ** 9.755389e-03
     timers/immediate.js type="depth1" thousands=2000       17.09 %        *** 7.176229e-04
     timers/set-immediate-breadth-args.js millions=5        10.63 %          * 4.250034e-02
     timers/set-immediate-breadth.js millions=10            20.62 %        *** 9.150439e-07
     timers/set-immediate-depth-args.js millions=10         17.97 %        *** 6.819135e-10

Backport-PR-URL: #18179
PR-URL: #17064
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks: deprecate undocumented API
025b9f2 
Backport-PR-URL: #18179
PR-URL: #16972
Refs: #14328
Refs: #15572
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@addaleax @MylesBorins
src: introduce internal C++ SetImmediate() mechanism
bd47272 
Backport-PR-URL: #18179
PR-URL: #17117
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@addaleax @MylesBorins
src: remove async_hooks destroy timer handle
e674488 
Backport-PR-URL: #18179
PR-URL: #17117
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
trace_events: add executionAsyncId to init events
ae4428e 
async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

Backport-PR-URL: #18179
PR-URL: #17196
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_wrap: add provider types for net server
6d55a4c 
Adds `TCPSERVERWRAP` and `PIPESERVERWRAP` as provider types. This
makes it possible to distinguish servers from connections.

Backport-PR-URL: #18179
PR-URL: #17157
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks: rename initTriggerId
cce92cc 
rename initTriggerId to defaultTriggerAsyncId such it matches the rest
of our naming.

Backport-PR-URL: #18179
PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks: separate missing from default context
03873db 
When context is missing the executionAsyncId will be zero. For the
default triggerAsyncId the zero value was used to default to the
executionAsyncId. While this was not technically wrong because the
functions are different themself, it poorly separated the two concepts.

Backport-PR-URL: #18179
PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks: use scope for defaultTriggerAsyncId
5a199a9 
Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.

Backport-PR-URL: #18179
PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@addaleax @MylesBorins
src: remove unused async hooks methods
089f18e 
Backport-PR-URL: #18179
PR-URL: #17757
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: vdeturckheim <vlad2t@hotmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@maclover7 @MylesBorins
async_hooks: use CHECK instead of throwing error
0f7c898 
SetupHooks is only available via `process.binding('async_wrap')`, so
there's no reason it shouldn't be called with the appropriate arguments,
since it is an internal-only function. The only place this function is
used is `lib/internal/async_hooks.js`.

Backport-PR-URL: #18179
PR-URL: #17832
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@addaleax @MylesBorins
async_hooks: use typed array stack as fast path
086af68 
- Communicate the current async stack length through a
  typed array field rather than a native binding method
- Add a new fixed-size `async_ids_fast_stack` typed array
  that contains the async ID stack up to a fixed limit.
  This increases performance noticeably, since most of the time
  the async ID stack will not be more than a handful of
  levels deep.
- Make the JS `pushAsyncIds()` and `popAsyncIds()` functions
  do the same thing as the native ones if the fast path
  is applicable.

Benchmarks:

    $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter next-tick process | Rscript benchmark/compare.R
    [00:03:25|% 100| 6/6 files | 20/20 runs | 1/1 configs]: Done
                                                   improvement confidence      p.value
     process/next-tick-breadth-args.js millions=4     19.72 %        *** 3.013913e-06
     process/next-tick-breadth.js millions=4          27.33 %        *** 5.847983e-11
     process/next-tick-depth-args.js millions=12      40.08 %        *** 1.237127e-13
     process/next-tick-depth.js millions=12           77.27 %        *** 1.413290e-11
     process/next-tick-exec-args.js millions=5        13.58 %        *** 1.245180e-07
     process/next-tick-exec.js millions=5             16.80 %        *** 2.961386e-07

Backport-PR-URL: #18179
PR-URL: #17780
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
trace_events: stop tracing agent in process.exit()
1d3d1dd 
Backport-PR-URL: #18179
PR-URL: #18005
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@apapirovski @MylesBorins
async_hooks: update defaultTriggerAsyncIdScope for perf
5dab90b 
The existing version of defaultTriggerAsyncIdScope creates an Array
for the callback's arguments which is highly inefficient. Instead,
use rest syntax and allow V8 to do that work for us. This yields
roughly 2x performance for this particular function.

Backport-PR-URL: #18179
PR-URL: #18004
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks,http: set HTTPParser trigger to socket
91d4eb5 
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.

Backport-PR-URL: #18179
PR-URL: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@AndreasMadsen @MylesBorins
async_hooks,test: only use IPv6 in http test
0211175 
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

Backport-PR-URL: #18179
PR-URL: #18143
Fixes: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 439112a..0211175

@addaleax addaleax mentioned this pull request Jan 22, 2018
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@jasnell jasnell jasnell approved these changes

Assignees

@gibfahn gibfahn

Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.