src: add async context frame to AsyncResource

[Flarna]

Add member to hold the async context frame to AsyncResource to avoid the need for the async_resource_context_frames_ map in env.

Semver major because it changes ABI.

[Flarna]

fyi @Qard If you think it's too early to add this semver major change in the async context frame area let me know.
I would prefer to do this before 23 is branched but latest before 24.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.89%. Comparing base (741004a) to head (ec65534).
Report is 64 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54879      +/-   ##
==========================================
- Coverage   87.89%   87.89%   -0.01%     
==========================================
  Files         651      651              
  Lines      183363   183348      -15     
  Branches    35710    35714       +4     
==========================================
- Hits       161170   161154      -16     
- Misses      15464    15468       +4     
+ Partials     6729     6726       -3     

Files with missing lines	Coverage Δ
src/api/async_resource.cc	95.23% <100.00%> (-0.60%)	⬇️
src/env-inl.h	96.73% <ø> (+0.16%)	⬆️
src/env.h	97.87% <ø> (ø)
src/node.h	95.74% <ø> (ø)

... and 27 files with indirect coverage changes

[Qard]

Generally LGTM, but I would like to wait until we can have some time to verify correctness of the AsyncContextFrame behaviour with customer testing before we commit any major ABI changes. Particularly we should be careful about this because of the feature being flag switched with the prior design.

[Flarna]

Do you expect this testing before 23 release?

[Qard]

Unlikely. This will exist behind a flag for a while yet. I'm mainly concerned about if we impact the unflagged behaviour negatively while this is still being evaluated.

Also, the flag would not be going away at any point, it just may swap its default as Electron also depends on ContinuationPreservedEmbedderData so they would need a way to opt out. For this reason we will need to be sure we don't break the original behaviour.

[Flarna]

Well, the impact is there before and after this PR as AsyncResource creates a v8:Global pointing to async_context_frame::current() which always calls isolate->GetContinuationPreservedEmbedderData().

If target is to avoid side effects/overhead we like should add some more if (env->options()->async_context_frame) to avoid calling
async_context_frame::current() and avoid to create async_context_frame::Scope instances.

I do not see the ABI change itself as critical as new majors have a ABI changes anyway. API is not changed.

But well, maybe delay this tuning till the next LTS. I don't expect a significant impact anyway.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[Qard]

I won't block if we feel we want to land sooner. I just wanted to be sure we aren't making ABI changes if they are unwanted. If there are other ABI changes happening anyway then probably fine to just land this.

[Flarna]

Every major has a different ABI simply because of v8 changes.
napi is built for ABI stability across major versions and this is not effected here.

My question to delay was more towards backporting/improving. Any difference between 23 and older could complicate this.

I think it's better to wait at least till 24 to avoid complications. So closing this for now.