Fix a replay issue where the 500ms timeout was being hit #1320
Conversation
- 3.7K nodes being inserted, with ~200 <tr>s as the root nodes making up the `resolveTrees` - each table row was dependent on the next, resulting in cascading failures; 19K repopulation of queue due to `nextNotInDOM`, and an additional cascading 350K repopulation of queue due to missing parent nodes
🦋 Changeset detectedLatest commit: 2525e47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const rootTree = putIntoMap(mutation, null); | ||
| if (rootNexts.has(mutation.node.id)) { | ||
| // some existing tree can't be inserted until this one is (nextNotInDOM check) | ||
| queueNodeTrees.unshift(rootTree); |
There was a problem hiding this comment.
Word of caution here - shift is O(n) which makes this operation O(n^2) now. Since this seems to be a hot path, you might be better off using a different data structure here
There was a problem hiding this comment.
I think the assumption here was that queueNodeTrees will be small in comparison to the tree data structure within each one. At any rate, the queueToResolveTrees function is there to put some organisation on the nodes in order to forestall the pathological cases elsewhere (the appendNode and the fact that appendNode can fail and put things back on the queue which causes the whole while (queue.length) { bit to brute force again and again)
So I think this is a minor concern; it could be replaced by a doubly linked list but my assumption would be that it's not a hot path.
There was a problem hiding this comment.
Seems reasonable, I just wanted to highlight it :)
<tr>s as the root nodes making up theresolveTreesarraynextNotInDOM, and an additional cascading 350K repopulation of queue due to missing parent nodes as algorithm attempted to continue over children