facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.98k stars 46.86k forks source link

Proposal: Get rid of pooling in synthetic event system #6190

Closed zpao closed 3 years ago

zpao commented 8 years ago

After staring at the confusing mess that is the synthetic event system today and talking with @sebmarkbage, we're no longer confident that we need the pooling that the system currently uses. GCs have gotten pretty good so we may be experiencing diminishing returns at this point, or maybe losing out on some benefits. We do have to consider that we still support some older browsers who's GCs don't have the benefit of the last several years worth of innovations so it might be premature, but it's probably worth investigating.

jimfb commented 8 years ago

:+1: We can also get rid of event.persist() which has caused engineers to bang their heads against walls for years. I think the value of persistent events shouldn't be underestimated, so I'm in favor of doing this even at the cost of slowing down legacy browsers - so long as things are acceptable in the latest versions of browsers.

zpao commented 8 years ago

Yes, persist() would go away naturally as a result (you'd be beholden to normal GC rules).

quantizor commented 8 years ago

Yasss. This is a strong ergonomic improvement in general.

iamdustan commented 8 years ago

I agree that for desktop devices and some mobile use cases this will not be an issue. Though I strongly suspect that removing this will cause noticeable GC pauses in touchmove events or other gestures on mobile devices.

While building a previous project (few years old now) we ran into issues while generating a lot of garbage while transforming shapes with gestures on iOS. Considering that JSCore is generally more optimized on mobile than V8, I suspect this will be more noticeable on Android devices.

I’m still interested in seeing the reality of what the GC cost would be without pooling.

taion commented 8 years ago

Is there something that causes non-persistent events to not satisfy the generational hypothesis the way it seems like they should?

Are the relevant GC pauses really in young gen?

sebmarkbage commented 8 years ago

Not all JS GCs work the same. This would optimize for a copying collector in which case this would be essentially free, except it might cause more frequent young GCs which should be fine. If the JS engine chose a different strategy that wouldn't be the case.

It's not about GC in general or whether the GC is optimized or not. It is whether it uses a copying collector specifically or not.

gaearon commented 3 years ago

We did this in 17.