-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Transferrable AbortController is very slow #43160
Comments
Even better if we could somehow detect the possible use of transferrable and then dynamically enable it. |
A quick search on GitHub showed that AbortController is not transferable by the spec: whatwg/dom#438. I propose we:
|
I think then we can implement AbortController in pure js? @benjamingr |
The one who pushed for this was @jasnell so I'd want him to weigh in but sgtm. |
Sorry for being a bit disconnected. We discussed this issue at the Collab Summit in Austin. We can remove the ability to transfer the AbortController. It is not part of the standard so there's good justification there. It was added as a convenience but given the performance hit, it's fine to remove that. We don't need a separate That said, I do plan on opening an issue in the DOM spec about making |
Let's recap the plan:
|
Have I missed anything @jasnell? |
There's really no reason to implement util.transferableAbortController in c++. Just use the makeTrsnsferable util with a regular AbortController. You can even keep the existing kClone/kDeserialize methods on the existing AbortController class. |
PR opened #43388 |
fixed in #44048 |
The way
AbortController
is implemented (with transferrable support) makes it very slow to use and I'm currently recommending everyone to use the npm package (abort-controller) instead.I don't think transferrable use of abort controller is very common and it's a bit unfortunate that such an unusual use case has significant performance impact on the common use.
Is there any way to improve this? As far as I understand it's not possible to improve the current implementation with transferrable support without further features from V8. Which leaves the question on whether we can add an option to disable/enable the transferrable implementation? That way users that don't need it can opt-out and get better performance.
The text was updated successfully, but these errors were encountered: