async_hooks: add AsyncLocal API mk2
-
Depends on #30959. See the last commit to understand what this PR changes- As #30959 was merged, this PR is ready for review
- Combines ideas for #27172 and #26540. I believe that this PR combines strong points of those implementation and builds a solid foundation for further evolution
- Includes tests and the benchmark (see changes in
benchmark/async_hooks/async-resource-vs-destroy.js
)
Introduces new AsyncLocal API to provide capabilities for building continuation local storage on top of it.
The implementation is based on async hooks
(hook callbacks + executionAsyncResource()
), but async hooks are not exposed in public API in any way. Public API is inspired by ThreadLocal class in Java.
I've marked @Flarna as a co-author, as the idea of this PR came out of #27172.
FYI @Quard, @Flarna, @vdeturckheim
Design overview
Main properties of the new API:
- Both API and implementation are very simple (
.get()
/.set()
+ optional.destroy()
methods)- This API achieves the same feature set as any other CLS API. Moreover, as it's quite "low-level", it's possible to build more opinionated APIs on top of
AsyncLocal
, say,continuation-local-storage
-like API orAsyncContext
(#26540). See sample snippets from the following comment: https://github.com/nodejs/TSC/issues/807#issuecomment-578384969
- This API achieves the same feature set as any other CLS API. Moreover, as it's quite "low-level", it's possible to build more opinionated APIs on top of
- It's relatively safe (memory-wise):
- Does not depend on
destroy
hook. Thus, misbehavingAsyncResource
won't be able to lead to mem leaks -
.destroy()
method disables the hook callback and frees all values from memory. Thus, it's possible to remove all remains of theAsyncLocal
- Does not depend on
- Has copy-on-write semantics, i.e. setting a new value branches subsequent parts of the tree
- Performance should be at least as good as for other implementations
Benchmarks against alternatives
I have modified async-resource-vs-destroy.js
benchmark and compared proposed implementation with #26540. Here is the result that I got on my machine:
$ ./node benchmark/async_hooks/async-resource-vs-destroy.js benchmarker=autocannon
async_hooks/async-resource-vs-destroy.js n=1000000 method="callbacks" type="async-local" benchmarker="autocannon": 20,277.6
async_hooks/async-resource-vs-destroy.js n=1000000 method="async" type="async-local" benchmarker="autocannon": 15,877.6
async_hooks/async-resource-vs-destroy.js n=1000000 method="callbacks" type="async-context" benchmarker="autocannon": 16,922.41
async_hooks/async-resource-vs-destroy.js n=1000000 method="async" type="async-context" benchmarker="autocannon": 11,841.2
async_hooks/async-resource-vs-destroy.js n=1000000 method="callbacks" type="async-resource" benchmarker="autocannon": 23,582.4
async_hooks/async-resource-vs-destroy.js n=1000000 method="async" type="async-resource" benchmarker="autocannon": 18,407.2
As expected, AsyncLocal
is significantly faster than AsyncContext
, but slower than the sample implementation that stores values as resource object properties (async-resource
type). As for the comparison with latter, I think that .destroy()
method that frees all values from the memory is more valuable than certain performance gain.
Benchmarks against existing libraries
A performance comparison with cls-hooked
v4.2.2 was also made. The benchmark is available here: https://gist.github.com/puzpuzpuz/f0b23458a821d7edab3738550e58f0e2 (again, it's a fragment of async-resource-vs-destroy.js
).
Here is the result:
$ ./node benchmark/async_hooks/async-resource-vs-destroy.js benchmarker=autocannon type=cls-hooked
async_hooks/async-resource-vs-destroy.js n=1000000 method="callbacks" type="cls-hooked" benchmarker="autocannon": 13,299.6
async_hooks/async-resource-vs-destroy.js n=1000000 method="async" type="cls-hooked" benchmarker="autocannon": 10,580.4
With these results cls-hooked
shows worst results among all candidates. But if both ns.bindEmitter
calls are commented (which contradicts with library guidelines for middlewares), the result improves significantly:
$ ./node benchmark/async_hooks/async-resource-vs-destroy.js benchmarker=autocannon type=cls-hooked
async_hooks/async-resource-vs-destroy.js n=1000000 method="callbacks" type="cls-hooked" benchmarker="autocannon": 19,704.8
async_hooks/async-resource-vs-destroy.js n=1000000 method="async" type="cls-hooked" benchmarker="autocannon": 14,580
With this result, cls-hooked
is on par with AsyncLocal
.
Possible enhancements
- Increase performance by storing values in resources directly instead of an internal
WeakMap
That should bring AsyncLocal
's performance to async-resource
's level (see benchmark results above). The downside is that it won't be possible to free values in all reachable resources in .destroy()
method anymore, so this method will provide weaker guarantees after the change.
- Introduce a global registry instead of public constructor
With current implementation, if a strong reference to AsyncLocal
is destroyed in application code, a strong reference in async_hooks
will still remain and the value propagation will continue to be executed. Thus, the instance will be effectively leaked.
Note: the same consideration also applies to async_hooks.createHook
. Once the user-land reference to an AsyncHook
is lost, it's not possible to disable it.
To make the API less error-prone AsyncLocal
's local constructor could be kept private and createAsyncLocal(string)
/destroyAsyncLocal(string)
functions could be introduced to async_hooks
. Each AsyncLocal
would be identified by a string (the exact type of id is not that important) and multiple calls of createAsyncLocal
with the same identifier would return the same AsyncLocal
instance.
The main problem with this enhancement is user-land library isolation, i.e. library A shouldn't be able to retrieve library B's instance.
Alternatively, option 1 from comment https://github.com/nodejs/node/pull/26540#issuecomment-588413146 can be considered. See this PR for draft implementation and benchmark results: https://github.com/vdeturckheim/node/pull/2
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
documentation is changed or added -
commit message follows commit guidelines