Adding event listeners for every selection change

[dhananjeyan-rm-17407]

While testing the performance of the editor, found that for every key press inside the editor we seem to bind and unbind event listeners twice or thrice, even though we unbind the events properly, it takes time for V8 engine to run the Garbage collector process, as a result of which till it is GC collected the removed event listeners stays in the JS memory, all these can be seen in the below attached video. So once GC is completed, we can see the number of event listeners returns back to normal.

https://github.com/ProseMirror/prosemirror/assets/151849447/893699bf-da64-4f75-b7ee-21f22d423d94

So as far as I debugged, I found that this and this line are 50% responsible for this issue, I'm not able to find the remaining causes for the issue. As far as I have debugged I think instead of binding and unbinding event listeners every time we can set a flag true or false for each case and proceed.

Could we start using flags instead of this event binding and unbinding approach @marijnh ?

[marijnh]

I don't think a event listener produce enough garbage collected memory to be noticeable at all, if this happens a few times per second. Are you just looking at that dark green number moving or do you have evidence that this is actually causing real issues?

[bZichett]

@dhananjeyan-rm-17407 I have yet to use that tool in chromium. I just tried it out and it's really great for auditing in a different way than the snapshot approach I was familiar with.

Additionally, you'll see that monitoring DOM nodes in that tool, the same type of thing is going on - GC eventually clears them out (you can test with pasting hundreds of nodes and deleting them)

@marijnh The performance on my computer during these GC phases seem negligible, but I'm on a powerful desktop - and this was on the prosemirror demo page, not a complex web app. I wonder how it is on less privileged computers (or phones); perhaps shaving off something like the GC phase of 1000+ objects every so often, it seems kind of frequently) might be worth testing.

Regards the same GC on DOM nodes; these are mandatory, and are less often. As long as there is no leak then all is fine. But if the library can reuse event listeners for selectionchange, or implement it in some singular way, and forego unnecessary GC, perhaps it may be worth it (if simple enough for equivalent function)

[joelewis]

This shouldn't be an issue at all, unless we are on less powerful devices where frequent GC could cause noticable issues.

That said, maintaining the same event handler (for frequent events like cursor movements/selection changes) does sound like a decent approach to avoid GC overload. I'd be happy to submit a PR if @marijnh is convinced it's worth a shot.

[marijnh]

selectionchange events are fired asynchronously, so it wouldn't be straightforward to determine whether one originates from a point where we turned them off (because we're messing with the DOM or selection) or right after that. Since the problem here seems mostly theoretical, I'm going to close this. Feel free to add information if an actual measurable issue is detectable.