perf(genadds) traverse children in reverse order#1398
perf(genadds) traverse children in reverse order#1398mdellanoce wants to merge 1 commit intorrweb-io:masterfrom
Conversation
🦋 Changeset detectedLatest commit: ecabd34 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 |
bebcb75 to
ecabd34
Compare
|
this might have an impact on performance when things do make it to the addList though, since the node order is reversed, and addList is traversed from the tail node to the head. I haven't found a case where that happens yet... Update: i think this^ shouldn't be a problem b/c the addList already accounts for next/previous sibling relationships |
|
You ran the ./packages/rrweb/test/benchmark/dom-mutation.test.ts benchmark? The only possible objection I can think of is whether this could makes things worse for different types of data sets, but really your write-up makes a great case that the existing approach is worst-case. |
|
For now thinking we will abandon this PR, it seems to most likely be addressed anyways by #1277 and so we can always revisit it after that one goes through |
I noticed when you add a deep/complex tree as part of a mutation, a large portion of the tree ends up being added to the addList. This is because genAdds traverses the children of each node in order, but serialization in pushAdd wants each node to have a serialized parent and nextSibling. Reversing the order of traversal in genAdds makes it more likely to avoid adding nodes to the addList.
I ran the benchmarks before/after and this was a pretty small improvement across the board.