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

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

Open
zpao opened this Issue Mar 4, 2016 · 6 comments

Comments

Projects
None yet
7 participants
@zpao
Member

zpao commented Mar 4, 2016

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

This comment has been minimized.

Show comment
Hide comment
@jimfb

jimfb Mar 4, 2016

Contributor

👍 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.

Contributor

jimfb commented Mar 4, 2016

👍 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

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 4, 2016

Member

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

Member

zpao commented Mar 4, 2016

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

@probablyup

This comment has been minimized.

Show comment
Hide comment
@probablyup

probablyup Mar 4, 2016

Contributor

Yasss. This is a strong ergonomic improvement in general.

Contributor

probablyup commented Mar 4, 2016

Yasss. This is a strong ergonomic improvement in general.

@iamdustan

This comment has been minimized.

Show comment
Hide comment
@iamdustan

iamdustan Mar 7, 2016

Contributor

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.

Contributor

iamdustan commented Mar 7, 2016

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

This comment has been minimized.

Show comment
Hide comment
@taion

taion Apr 10, 2016

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?

taion commented Apr 10, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Apr 11, 2016

Member

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.

Member

sebmarkbage commented Apr 11, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment