Skip to content

async_hooks: add AsyncLocal API mk2

Rodrigo Muino Tomonari requested to merge github/fork/puzpuzpuz/async-local into master
  • 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 or AsyncContext (#26540). See sample snippets from the following comment: https://github.com/nodejs/TSC/issues/807#issuecomment-578384969
  • It's relatively safe (memory-wise):
    • Does not depend on destroy hook. Thus, misbehaving AsyncResource 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 the AsyncLocal
  • 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

  1. 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.

  1. 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), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports

Loading