Remove expensive update all notes call

[cutandpastey]

This should go some way to alleviating the unresponsive script error some users are getting on heavily noted content. Would appreciate some eyes from @robinedman on this one, may be the case there's other quick wins in this area.

[robinedman]

I'd be a bit wary of doing this since it breaks functionality (prevents merging from happening when the user removes text in between two notes). This use case is probably important but not critical: so as a quick fix that'll be followed by a better solution I'd say it can be worth it.

Instead of this line, however, I would comment out the call to mergeIfNecessary itself. I think that's a lot clearer. And it'd be good to have a comment above describing why it's currently commented out.

[hmgibson23]

My gut feeling is that the tree traversal is the expensive operation so while this will helps us out, I'm not sure it's the perfect fix. But agree that if it makes things easier temporarily at the cost of a feature then it's probably for the best.

\cc @sihil @alastairjardine @rcrphillips

[robinedman]

You beat me too it! Yes, I looked a bit more at the code. While I haven't done any benchmarking, it seems to me like it's vdom.findAllNotes(treeFocus) that is the expensive operation here.

We're running that twice for every time we run ensureNoteIntegrity (once in mergeIfNecessary and once in updateNoteBarriers). If we did it directly inside ensureNoteIntegrity and passed it into the other functions we'd save one call.

Also, we've already throttled ensureNoteIntegrity, but clearly that isn't enough. I'd suggest having a look at findAllNotes as well as other similar traversals we do inside of ensureNoteIntegrity, and see if they can either be optimised. Or if we can avoid traversing the entire virtual dom.

[robinedman]

Closing this one as I'm pretty sure these issues were sorted