Repository navigation
Conversation
With the AsyncContextFrame implementation, every AsyncLocalStorage.run() built two copies of the current frame: one to enter the new value and another one in the finally block to put the previous value back. Both copies went through the Map iterator protocol, which calls next() and allocates an entry array per store. Copy frames with forEach() instead, and when fn() neither replaced the current frame (enterWith()) nor removed a store from it (disable()), put back the frame that was current before run() instead of building a new one. Otherwise keep building a new frame as before, so that those changes stay visible after run() returns. getStore() now does a single lookup unless the stored value is undefined. A side effect is that after a top-level run() returns the current frame is undefined again, as it was before run(), instead of a frame that holds the previous value of the store. getStore() returns the same values either way. Measured locally: als.run(i, noop) 100 ns -> 16 ns als.run(i, noop), two other stores set 145 ns -> 35 ns await als.run(i, async fn with 2 awaits) 250 ns -> 89 ns async_hooks/async-local-storage-run.js 7.0M -> 20M ops/s Assisted-by: claude:opus-5.5 Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
als.run(i, noop)als.run(i, noop), two other stores setawait als.run(i, async fn)with two awaitsbenchmark/async_hooks/async-local-storage-run.jsAsyncLocalStorage.run()callsenterWith()twice, once to enter and once infinallyto restore, and each call builds a new frame:node/lib/internal/async_local_storage/async_context_frame.js
Lines 54 to 70 in 2aef805
Each new frame copies the current one through the Map iterator protocol:
node/lib/internal/async_context_frame.js
Lines 12 to 16 in 2aef805
Frames are now copied with
forEach()instead, andrun()puts the previous frame back instead of building a new one when possible.