Proposal for reworking promise integration into async_hooks
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 20/100
- issue の種類
- 機能追加
- 明瞭さ
- 説明が足りない
- 活発さ
- 停滞
- 技術スタック
- node.js
調査の方向性
Start by reviewing the existing async_hooks promise integration and the proposed promise_hooks module, then examine how [[then]] and async/await are handled. The open question about a custom MicrotaskQueue and the linked diagnostics issue must be resolved; done means a defined integration design that addresses compatibility and can be scheduled for a major release.
索引モデルが issue の本文から書いたものです。
説明
Background
The current implementation of promise integration into async_hooks is.
- For each new
[[Promise]]object, call theinithook with aPromiseWrapreferencing that Promise. - At
[[resolve]]or[[reject]]call theresolvehook. - For each
[[then]]callback, call thebeforehook before calling the callback. - For each
[[then]]callback, call theafterhook after calling the callback. - When the Promise object is garbage collected call the destroy hook.
I think this implementation is fundamentally wrong, as it intertwines the promise lifecycle with async_hooks. This causes a number of issues that are currently blocking us from making async_hooks stable.
- Performance issues caused by listening for the garbage collection event.
- Thenables are not tracked when used by a native function that creates a microtrask.
destroyhook is not called ifasync_hookis enabled after Promise creation.- Tracking the async boundary when multiple
.then()calls are used on the same promise object, is not possible.
Proposal
My proposal is to rework the promise integration into async_hooks such that the async barrier is around the [[then]] call, not creating a new Promise object.
- at the call of
[[then]]on apromiseorthenablecreate a resource object (or use the promise/thenable object created by[[then]]) then call theinithook. - at the call of the
[[then]]callback, thebeforehook is called. - at the end of the
[[then]]callback, call theafterhook, immediately after call thedestroyhook.
How it solves the above-mentioned issues
Performance issues caused by listening for the garbage collection event.
Because the before and after hooks are only called once per resource, the destroy hook can be called immediately after the after hook. Thus completely eliminating the need to track promise objects in the garbage collector.
Thenables are not tracked when used by a native function that creates a microtrask.
This can now be solved because we don't need to know when the object was created or destroyed. The only knowledge that is required is when the [[then]] method of the thenable is called by the native JS APIs which invokes the microtask queue. That is actually doable, as we could hook into those APIs. Only manual calls to [[then]] on a thenable will not be tracked. But I don't see that as a concern, because that is not an async action.
destroy hook is not called if async_hook is enabled after Promise creation.
Again, because the destroy hook is called with the after hook, calling the destroy hook becomes trivial.
Tracking the async boundary when multiple .then() calls are used on the same promise object, is not possible.
This directly confronts this issue, by making the async boundary the [[then]] call.
Tracking the lifetime of promises
This proposal removes features from async_hooks that provide insight into promises. That information is still valuable.
To keep providing that information, I propose making a dedicated promise_hooks module. A user can then connect the promise lifecycle with async_hooks via a promiseId that is exposed both in promise_hook and via the resource in async_hooks.
Compatability
This does change the default async chain, as the start point of the async-barrier is now the .then() call and not the new Promise call. However, I think this is actually a preferred default. For example, lazyloading a resource with new Promise, would currently not be tracked correctly, but with the proposed change it will.
Even though this changes the async graph, the proposed async graph will still be valid and useful in most cases. And the existing async graph can be restored by integrating the proposed promise_hooks module.
implementing this proposal should be part of a major release.
Open questions
- It is unclear to me how this would integrate with
async/await. As I don't have a good grasp of how the internal[[then]]calls are used and wrapped. It has been proposed that we supply our own customMicrotaskQueue(https://github.com/nodejs/diagnostics/issues/376#issuecomment-623682088) to solve this. I think this could also be a good approach to hook into the native promise APIs, which will be necessary to track[[then]]calls on thenables.
- 主要言語
- 言語のデータがありません
- スター
- 550
- フォーク
- 69
- PR マージ指標
- 30日以内にマージされた PR はありません
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
nodejs/diagnostics のほかの issue
-
難易度 1/5 1時間未満 初心者へのやさしさ 65/100
nodejs/diagnostics#648 · コメント 3 件 ·
-
難易度 1/5 1時間未満 初心者へのやさしさ 20/100
nodejs/diagnostics#690 · リアクション 1 件 ·
-
難易度 1/5 1時間未満 初心者へのやさしさ 20/100
nodejs/diagnostics#689 ·
-
難易度 1/5 1時間未満 初心者へのやさしさ 25/100
nodejs/diagnostics#688 ·
-
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
nodejs/diagnostics#687 ·
nodejs/diagnostics の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
enhancement
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
OpenwaterHealth/openmotion-bloodflow-app#604 · コメント 1 件 ·
-
good first issue
難易度 1/5 1時間未満 初心者へのやさしさ 90/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
rust-bitcoin/rust-bitcoin#6930 · コメント 1 件 ·
-
Solved site promotion gate fails on runner PHP patch drift (expects 8.2.33, runner installs 8.2.34) オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
Automattic/blocks-engine#2161 ·