Repository navigation
AbortSignal pattern is slow #44
Description
Activity
this was recently addressed. The change may not have been fully backported tho? Not sure.
this was recently addressed. The change may not have been fully backported tho? Not sure.
Which particular pull request are you referring to?
#44048 looks like it was backported to 18.x in #44941 tho
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...
Reacted by Toni VillenaInteresting 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.
@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.abortednow 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.Reacted by Robert Nagy, Debadree Chatterjee and Carlos FuentesReferencing: nodejs/node#46648
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.
Reacted by Robert Nagy, Marvin Hagemeister, Carlos Fuentes and Vinicius LourençoClosing since it's a duplicate.
- added a commit that references this issue
on May 7, 2023
To the point that e.g. RxJS has decided not support it.
ReactiveX/rxjs#6675 (comment)