What's the status of async_local_storage scope implementation?

[zen0wu]

Given async_hooks scope has pretty bad performance (reference #695), it seems that async_local_storage can be used. Looking at the implementation, it's a much easier/clearer approach.

Based on my own local test, async_local_storage basically has no performance impact as if I set enabled: false, while async_hooks slowed down the program by about 2x.

But I cannot see any documentation around async_local_storage, nor it's part of index.d.ts's type definition of the scope config (and plus the doc suggest against setting scope in the first place), so I'm wondering what's the status of that implementation? Is it recommended at all? Has anyone used it in production?

@Qard @rochdev Mentioned people on the original PR 🙏

[Qard]

The decision was to make it an option initially as, at least at first, there was no solution to the thenables issue in V8 at the time of merging. The async_hooks scope manager has its own hack to fix it within the agent, but I later made a fix directly to V8. If you are using Node.js 14.x then AsyncLocalStorage should just work for basically anything you need it for. The backport of the fix has not yet reached 12.x though.

Because the difference between the two scope managers is relatively significant it seemed best to at least give a bit of time with AsyncLocalStorage as an option rather immediately switch the default, just to make sure it doesn't have any unforeseen issues that could break many production users.

To be clear: if you are on 14.x, it should be perfectly safe to use AsyncLocalStorage, as far as I can tell.

[zen0wu]

@Qard Thanks for the context!

Allow me double check my understanding.

• There's a bug in V8 around thenable, which affects both async_hooks and async_local_storage scope
• async_hooks has some hack to fix it, but async_local_storage does not
• It's fixed by your change upstream in V8 (could you point me to it), and merged into 14.x (is it >=14.5 based on the code in async_hooks.js?)

Unfortunately we're on 12.x, and pre 12.17.0, which means we don't get AsyncLocalStorage, but I'm considering upgrading it. This all depends on whether we get the fix on later version. If it's on the V8 side, 12.x probably is not going to get it since my understanding is we don't upgrade V8 between minor versions.

General curiosity, could you help me understand a bit about the thenable bug, its impact and point me to the fix you made? Thanks again!

[Qard]

Yes, there was a bug in V8 which made thenables not emit proper async_hooks events. You can find a deeper explanation of the issue here: https://gist.github.com/Qard/faad53ba2368db54c95828365751d7bc

The async_hooks scope manager has its own hack within the agent to deal with the issue. There was not a good way to replicate the hack for the AsyncLocalStorage scope manager.

A fix has already been made to V8 and landed about two months ago. This is the fix targeting 14.x, which has landed: nodejs/node#33778 and this is the backport to 12.x, which has not yet landed: nodejs/node#34776 (The V8 fix was adapted to support the older V8 version which, in this particular case, was only minimally different)

AsyncLocalStorage has actually been backported to 12.x already, but because the V8 fix has not yet landed there, it's not entirely stable yet. You can play with it now if you want though. As long as you avoid modules that use thenables, or wrap them in real promises, it should work fine.

[zen0wu]

@Qard Thanks so much for the detailed explanation, the writeup is awesome!

I guess this issue can be closed now, the followup seem to be

• Wait for the upstream to land on Node 12
• Update the extra config type in index.d.ts
• Maybe also check for Node 12 version to enable async_local_storage scope?

I'll subscribe the Node PR. I'll leave this open a while in case people think it's useful for tracking, but feel free to close!

[Qard]

Just FYI: The fix landed in 12.x a few hours ago. 😄