process: preserve AsyncLocalStorage on queueMicrotask only when needed

[gurgunday]

Does the same optimization as #59873 for queueMicrotask

branch:

./node benchmark/run.js --filter queue-microtask process process/queue-microtask-breadth.js process/queue-microtask-breadth.js n=400000: 18,938,347.488090977 process/queue-microtask-depth.js process/queue-microtask-depth.js n=1200000: 35,862,646.06556887

main

./node benchmark/run.js --filter queue-microtask process process/queue-microtask-breadth.js process/queue-microtask-breadth.js n=400000: 5,198,220.087856157 process/queue-microtask-depth.js process/queue-microtask-depth.js n=1200000: 12,707,696.847598467

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.58%. Comparing base (6274eb7) to head (2f8776e).
⚠️ Report is 105 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #60913 +/- ## ========================================== + Coverage 88.55% 88.58% +0.02%  ========================================== Files 703 703 Lines 208291 208296 +5 Branches 40170 40166 -4 ========================================== + Hits 184443 184509 +66 + Misses 15874 15807 -67 - Partials 7974 7980 +6 

Files with missing lines	Coverage Δ
lib/internal/process/task_queues.js	100.00% <100.00%> (ø)

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gurgunday]

cc @nodejs/performance

WDYT?

[H4ad]

Before merging, some concerns raised in another PR with same changes:

One thing worth noting is that this would be a breaking change as currently the AsyncLocalStorage.bind(...) will always bind, even if there are no hooks or stores in use, but async_hooks could start listening at any time. In the current model you could start listening after the bind call there but before the callback is called and you would see the events. After this change you would not.
By @Qard

Would mind giving some thoughts on this one, @mcollina and @Qard?

Edit:
Benchmark: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1770/

[github-actions]

Failed to start CI

 ⚠ No approving reviews found ✘ Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/19980375054

[Qard]

@H4ad Yes, the same concern applies. It is, in my opinion, problematic that it was ever possible to start seeing events for a given task partway through its lifecycle rather than deciding at emit time if any of the events should be present. But that is certainly a breaking change and so should be marked as such. A desirable change, I think. But a breaking one nonetheless.

At the same time though, async_hooks is still marked as experimental, and AsyncLocalStorage no longer relies on it, so the break is likely a lot less critical. We may be able to allow it without a major bump. I'm not strongly opinionated either way.

[mcollina]

lgtm

[mcollina]

I'm ok in this being a minor, but I don't think we can backport it to 22.

[H4ad]

Benchmark result:

 confidence improvement accuracy (*) (**) (***) process/bench-env.js operation='delete' n=1000000 0.89 % ±5.64% ±7.51% ±9.77% process/bench-env.js operation='enumerate' n=1000000 1.65 % ±3.36% ±4.47% ±5.82% process/bench-env.js operation='get' n=1000000 -0.79 % ±1.68% ±2.24% ±2.91% process/bench-env.js operation='query' n=1000000 1.36 % ±2.64% ±3.51% ±4.57% process/bench-env.js operation='set' n=1000000 0.90 % ±3.45% ±4.59% ±5.98% process/bench-hrtime.js type='bigint' n=1000000 -1.11 % ±4.98% ±6.62% ±8.62% process/bench-hrtime.js type='diff' n=1000000 * -3.27 % ±3.21% ±4.27% ±5.57% process/bench-hrtime.js type='raw' n=1000000 -0.25 % ±3.02% ±4.02% ±5.24% process/coverage.js n=100000 0.03 % ±1.10% ±1.47% ±1.92% process/getActiveResourcesInfo.js n=100000 immediatesCount=10000 timeoutsCount=10000 requestsCount=10000 handlesCount=10000 -1.21 % ±5.47% ±7.28% ±9.48% process/memoryUsage.js n=100000 0.18 % ±0.41% ±0.55% ±0.71% process/next-tick-breadth-args.js n=10000000 0.05 % ±0.35% ±0.46% ±0.60% process/next-tick-breadth.js n=10000000 0.01 % ±0.75% ±1.00% ±1.30% process/next-tick-depth-args.js n=7000000 -0.79 % ±1.24% ±1.65% ±2.14% process/next-tick-depth.js n=7000000 0.48 % ±1.89% ±2.51% ±3.27% process/next-tick-exec-args.js n=4000000 -0.02 % ±8.70% ±11.57% ±15.06% process/next-tick-exec.js n=4000000 -0.03 % ±0.17% ±0.23% ±0.30% process/next-tick-loop-args.js loop=100 n=10000 -0.84 % ±2.36% ±3.14% ±4.08% process/next-tick-loop-args.js loop=100 n=20000 -0.43 % ±0.87% ±1.16% ±1.51% process/next-tick-loop-args.js loop=100 n=40000 0.35 % ±0.58% ±0.78% ±1.01% process/next-tick-loop-args.js loop=200 n=10000 0.29 % ±1.80% ±2.40% ±3.12% process/next-tick-loop-args.js loop=200 n=20000 0.29 % ±0.80% ±1.07% ±1.39% process/next-tick-loop-args.js loop=200 n=40000 -0.50 % ±0.64% ±0.85% ±1.10% process/next-tick-loop.js loop=100 n=10000 -0.73 % ±1.50% ±2.00% ±2.61% process/next-tick-loop.js loop=100 n=20000 -0.02 % ±1.85% ±2.46% ±3.21% process/next-tick-loop.js loop=100 n=40000 -0.24 % ±1.29% ±1.72% ±2.24% process/next-tick-loop.js loop=200 n=10000 -0.41 % ±1.30% ±1.73% ±2.26% process/next-tick-loop.js loop=200 n=20000 -0.27 % ±1.40% ±1.87% ±2.43% process/next-tick-loop.js loop=200 n=40000 0.19 % ±1.12% ±1.49% ±1.94% process/queue-microtask-breadth.js n=400000 *** 221.40 % ±5.59% ±7.53% ±9.98% process/queue-microtask-depth.js n=1200000 *** 137.38 % ±5.80% ±7.81% ±10.33% process/resourceUsage.js n=100000 -0.67 % ±2.84% ±3.77% ±4.91% Be aware that when doing many comparisons the risk of a false-positive result increases. In this case, there are 32 comparisons, you can thus expect the following amount of false-positive results: 1.60 false positives, when considering a 5% risk acceptance (*, **, ***), 0.32 false positives, when considering a 1% risk acceptance (**, ***), 0.03 false positives, when considering a 0.1% risk acceptance (***) 

Impressive 🚀

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/70406/

[mcollina]

lgtm

[Qard]

LGTM with one small nit.

[Flarna]

At the same time though, async_hooks is still marked as experimental, and AsyncLocalStorage no longer relies on it, so the break is likely a lot less critical. We may be able to allow it without a major bump. I'm not strongly opinionated either way.

The linked PR #59873 was marked semver major.

[nodejs-github-bot]

Landed in a65421a