Proposal for reworking promise integration into async_hooks
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 20/100
- Loại issue
- Tính năng
- Độ rõ ràng
- Cần làm rõ
- Mức độ hoạt động
- Đình trệ
- Công nghệ
- node.js
- Lĩnh vực
- developer-experience
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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.
- Ngôn ngữ chính
- Không có dữ liệu ngôn ngữ
- Star
- 550
- Fork
- 69
- Chỉ số merge pull request
- Không có pull request nào được merge trong 30 ngày
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của nodejs/diagnostics
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 65/100
nodejs/diagnostics#648 · 3 bình luận ·
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 20/100
nodejs/diagnostics#690 · 1 reaction ·
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 20/100
nodejs/diagnostics#689 ·
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 25/100
nodejs/diagnostics#688 ·
-
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 25/100
nodejs/diagnostics#687 ·
Tất cả issue của nodejs/diagnostics
Issue tương tự
-
area/install-update P2 tool/browser type/bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100
NousResearch/hermes-agent#122356 · 2 bình luận ·
-
bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 74/100
-
hackbot
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 64/100
-
⚡️ Performance: [no-misused-promises] computes void-returning parameters for calls with no arguments Đang mởpackage: eslint-plugin performance spam:exempt
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
typescript-eslint/typescript-eslint#12922 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100