Reduction in reconcilliation lookahead threshold

1

Hi,

We are experiencing extreme decreased performance when deleting multiline content at the top of a page with lots of content below it since prosemirror@1.20.2. We have debugged the code and noticed that the culprit seems to be in the findNodeMatch in the ViewTreeUpdater. It seems when we delete the rows, the findNodeMatch exits early when looping to find a matching node. This causes all the content below to have its NodeView’s recreated again instead of updating them which is extremely slow.

We have narrowed it down to this line prosemirror-view/viewdesc.js at master · ProseMirror/prosemirror-view · GitHub. We notice that you limit the lookahead in the children to just one which is not enough when deleting multiline content. We noticed that the threshold was also reduced in this commit Fix accidentally quadratic logic in node updating · ProseMirror/prosemirror-view@60a9a82 · GitHub and wondering why the threshold was reduced?

As a seperate question, do we need the threshold at all? Seems like it could potentially be an over optimisation.

1 Like
2

Hi Sean,

That change to the lookahead does seem suspicious (it makes it pointless to have a loop at all there, and I don’t remember why I made it) and might be an accidentally committed debugging change.

However, removing the threshold seems like it will make updates where the children are completely replaced with different children quadratic to update, since each child will scan through all the nodes after it.

In the situation you describe, a multi-node deletion at the start of a big node, findNodeMatch should be able to match all the nodes after the deletion, making updating cheap. I set up a simple test case and verified that, at least in my test, this is working.

So, though I’m definitely open to bumping the lookahead back up to 5, I would like to get to the bottom of the performance problem you’re having, since there may be another bug involved. Can you create a simple stand-alone script that produces the problem?

3

Hi Marijn,

Thanks for getting the threshold PR merged so quickly :pray:

We have spent the past few days looking into the issue. It turns out that we were not entering into the preMatch path of the reconcilliation algorithm. We noticed that it was failing as soon as we were doing the if (node != frag.child(fI - 1)) { break } which is obviously a quick memory check. To test we changed it to if (!node.eq(frag.child(fI - 1)) { break } and saw that the performance was restored, but obviously this is not a good fix as its slow. This led us to believe that there was something not right with our code.

It turns out that the issue is actually on our side were we are doing are replacing a node with itself when we shouldn’t be. Once we stopped that from happening, performance was restored :smiley: