AbortSignal pattern is slow #44

Activity

1. ronag

   added

   performance-agenda

   on Jan 20, 2023
2. anonrig commented on Feb 9, 2023

   

   anonrig

   on Feb 9, 2023

   Member

   More actions

   What can we do in this area? Any suggestions? @jasnell @mcollina
3. jasnell commented on Feb 9, 2023

   

   jasnell

   on Feb 9, 2023

   Member

   More actions

   this was recently addressed. The change may not have been fully backported tho? Not sure.
4. anonrig commented on Feb 9, 2023

   

   anonrig

   on Feb 9, 2023

   Member

   More actions

   this was recently addressed. The change may not have been fully backported tho? Not sure.

   Which particular pull request are you referring to?
5. jasnell commented on Feb 9, 2023

   

   jasnell

   on Feb 9, 2023

   Member

   More actions

   #44048 looks like it was backported to 18.x in #44941 tho
6. ronag commented on Feb 10, 2023

   

   ronag

   on Feb 10, 2023

   MemberAuthor

   More actions

   @jasnell do you mean the transferable fix? I think abort signals are still slow but now it's because of eventtarget.
7. ronag commented on Feb 10, 2023

   

   ronag

   on Feb 10, 2023

   Last edited by ronag

   MemberAuthor

   More actions

   ReactiveX/rxjs#6675 (comment)

   After using AbortSignal for a large project that was dealing with high-speed emissions, the performance of AbortSignal, in particular adding and removing event listeners for abort events, was so incredibly poor that I'm unwilling to introduce any flavor of it in RxJS.

   We were waiting and waiting for this and thinking maybe we'd break ground on it in 8.x. And now I can't say that I think it's a good idea.

   Use takeUntil(fromEvent(signal, 'abort')) in the meantime.

   There has a been a lot of focus here on creating signals and events as well as emitting events but that's not actually the problem for the RxJS case...

   👀React with 👀1Reacted by Toni Villena
8. ronag commented on Feb 10, 2023

   

   ronag

   on Feb 10, 2023

   MemberAuthor

   More actions

   @benjamingr
9. ronag

   changed the title [-]AbortSignal is slow[/-] [+]AbortSignal pattern is slow[/+] on Feb 10, 2023
10. jasnell commented on Feb 10, 2023

    

    jasnell

    on Feb 10, 2023

    Member

    More actions

    Interesting ok. Well, I guess we need to figure out how to speed up EventTarget now. That said, really none of the Web Platform standard apis were designed with performance as a high priority. We'll have to keep find ways of optimizing while still remaining compliant to the specs.
11. santigimeno commented on Feb 10, 2023

    

    santigimeno

    on Feb 10, 2023

    Member

    More actions

    @jasnell any thoughts on this and specially on the Event.isTarget property? Thanks!
12. benjamingr commented on Feb 10, 2023

    

    benjamingr

    on Feb 10, 2023

    Member

    More actions

    @benjamingr

    @ronag I'm reading this, I think EventTarget can be made faster (discussion in that thread + a few other ideas).

    As for RxJS's case, we can also fast-path util.aborted now that we have that. Basically when a user calls it we don't go through the addEventListener machinery and go through parallel machinery avoiding it.

    👍React with 👍3Reacted by Robert Nagy, Debadree Chatterjee and Carlos Fuentes
13. anonrig commented on Feb 20, 2023

    

    anonrig

    on Feb 20, 2023

    Member

    More actions

    Referencing: nodejs/node#46648
14. anonrig

    closed this as completedon Feb 20, 2023
15. ronag

    reopened this on Feb 20, 2023
16. ronag

    closed this as completedon Feb 20, 2023
17. ronag

    reopened this on Feb 20, 2023
18. benjamingr commented on Mar 7, 2023

    

    benjamingr

    on Mar 7, 2023

    Member

    More actions

    So - just to elaborate a little on RxJS's case. Us fixing EventTarget wouldn't help them at all since it would still be slow in the browser which they also support.

    Their avoidance isn't because our implementation is slow.

    👍React with 👍4Reacted by Robert Nagy, Marvin Hagemeister, Carlos Fuentes and Vinicius Lourenço
19. anonrig

    closed this as completedon Apr 3, 2023
20. anonrig commented on Apr 3, 2023

    

    anonrig

    on Apr 3, 2023

    Member

    More actions

    Closing since it's a duplicate.
21. debadree25

    added a commit that references this issue on May 7, 2023

    lib: reuse default DOMException in AbortController

    ...

    832efd8