Perf: optimize the performance of processMutations #1187

wfk007 · 2023-03-23T13:08:33Z

processMutations contains two phases:

processMutation
emit

and they have bad performance in some cases.

processMutation

case

In the case below: (reproduce link: https://github.com/wfk007/test-processMutation)

we create 1000 ElementNode and insert them into #target, each Node is the parent of the following Node

<html>
    <body>
        <button id="add-nodes">Add multi nodes</button>

        <div id="target"></div>

        <script src="https://cdn.jsdelivr.net/npm/rrweb@latest/dist/record/rrweb-record.min.js"></script>
        <script>
            const addNodes = document.querySelector('#add-nodes');
            const target = document.querySelector('#target');

            addNodes.addEventListener('click', () => {
                const total = 1000;
                let parent = target;
                for (let i = 0; i < total; i++) {
                    const el = document.createElement('div');
                    el.textContent = `el-${i + 1}`;
                    parent.append(el);
                    parent = el;
                }
            });
            rrwebRecord({
                sampling: {
                    mouseInteraction: false,
                    mousemove: false,
                    scroll: 300,
                    input: 'last',
                },
                slimDOMOptions: true,
                emit(event) {
                    console.log(event);
                },
            });
        </script>
    </body>
</html>

performance

WITHOUT record, it takes about 50ms to render

WITH record it takes 655ms and processMutation takes 604ms

reason

The dom structure below can be created in many ways:

body
    E1
       E2

Approach 1:

create E1
create E2
E1 append E2
body append E1

Browser Mutation observer callback result:

[
    {
        target: body,
        addedNodes: [E1],
        type: "childList",
    },
]

approach 2:

create E1
body append E1
create E2
E1 append E2

Browser Mutation observer callback result:

[
    {
        target: body,
        addedNodes: [E1],
        type: "childList",
    },
    {
        target: E1,
        addedNodes: [E2],
        type: "childList",
    },
]

In order to cover different Mutation observer callback results and track adds/removes Nodes, RRweb loops all mutations and its' addedNodes.
Each Node in addedNodes will still travel all its' childNodes.

Considering approach2 result:

loop array index 0:
travel E1
travel E2(which is the childNode of E1)
loop array index 1:
travel E2(no need to travel E2 and its' childNodes again)

solution

In genAdds, if the node is added to addedSet or movedSet, there is no need to travel it and its' children again

after optimizing, it takes 160ms and processMutation takes 77ms

emit

case

(case is on the way)

performance

when the size of addedSet is huge(20k+ Nodes), the for-loop is very slow with time complexity O(n^2) to find a valid Node

reason

There are 20000 nodes in addList, and index 900 is the first valid Node from right to left, but its' previous is not a valid Node. we loop from right to left to find the first valid Node

addList.get(19999): O(N) time complexity and it is very slow
addList.get(19998): O(N) time complexity and it is slow too
...
addList(900): find valid Node
899 is selected as candidate Node
899 is invalid
so we loop from right to left
repeat 1/2/3...

It is very very slow(takes about 16s).

solution

I create a linkedNodeList based on DoubleLinkedList to accelerate node query with array index, and query time complexity decrease from O(n) to O(1) with a huge performance boost

changeset-bot · 2023-03-23T13:08:36Z

🦋 Changeset detected

Latest commit: 3557938

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
rrweb	Patch
rrweb-snapshot	Patch
rrdom	Patch
rrdom-nodejs	Patch
rrweb-player	Patch
@rrweb/types	Patch
@rrweb/web-extension	Patch

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

Juice10 · 2023-04-07T09:52:10Z

Hey @wfk007 this is a great performance improvement, thank you so much for putting all the hard work into this.
One thing I'm missing is making sure your hard work doesn't get reverted by accident in the future.
We have a benchmark suite in the test suite that would be a great candidate for this, especially in combination with a GitHub action (that we don't have yet) https://github.com/benchmark-action/github-action-benchmark that'll make sure performance gets tested on each pull request.

eoghanmurray · 2023-04-14T15:05:26Z

I haven't looked too closely but the change is nice and small and seems to make sense.
I'm putting this out to a 'canary' environment today to see if any problems arise.
Note: The ESLint failure can be solved by rebasing upon the current trunk.

wfk007 · 2023-04-20T09:29:04Z

Hey @wfk007 this is a great performance improvement, thank you so much for putting all the hard work into this. One thing I'm missing is making sure your hard work doesn't get reverted by accident in the future. We have a benchmark suite in the test suite that would be a great candidate for this, especially in combination with a GitHub action (that we don't have yet) https://github.com/benchmark-action/github-action-benchmark that'll make sure performance gets tested on each pull request.

@Juice10 @eoghanmurray I have been trapped at work and complex trivial things in life recently, and I will add a benchmark test and fix the Lint error later, thanks for your review and great suggestions.

wfk007 · 2023-04-28T10:10:13Z

@Juice10 @eoghanmurray Hi, I plan to separate this PR into two, thanks for your great suggestions again

part one: #1214

part two: #1220

packages/rrweb/src/record/mutation.ts

wfk007 force-pushed the optimization-of-processMutation branch from 76aa2a7 to 7960f18 Compare March 24, 2023 02:07

optimize performance of processMutations

3557938

wfk007 force-pushed the optimization-of-processMutation branch from df1d889 to 3557938 Compare March 27, 2023 05:40

wfk007 changed the title ~~optimize processMutation~~ Perf: optimize the performance of processMutations Mar 27, 2023

YunFeng0817 self-requested a review April 27, 2023 13:26

wfk007 closed this Apr 28, 2023

YunFeng0817 reviewed Apr 30, 2023

View reviewed changes

packages/rrweb/src/record/mutation.ts Show resolved Hide resolved

packages/rrweb/src/record/mutation.ts Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perf: optimize the performance of processMutations #1187

Perf: optimize the performance of processMutations #1187

wfk007 commented Mar 23, 2023 •

edited by YunFeng0817

Loading

changeset-bot bot commented Mar 23, 2023 •

edited

Loading

Juice10 commented Apr 7, 2023

eoghanmurray commented Apr 14, 2023

wfk007 commented Apr 20, 2023 •

edited

Loading

wfk007 commented Apr 28, 2023 •

edited

Loading

Perf: optimize the performance of processMutations #1187

Perf: optimize the performance of processMutations #1187

Conversation

wfk007 commented Mar 23, 2023 • edited by YunFeng0817 Loading

processMutation

case

performance

reason

solution

emit

case

performance

reason

solution

changeset-bot bot commented Mar 23, 2023 • edited Loading

🦋 Changeset detected

Juice10 commented Apr 7, 2023

eoghanmurray commented Apr 14, 2023

wfk007 commented Apr 20, 2023 • edited Loading

wfk007 commented Apr 28, 2023 • edited Loading

wfk007 commented Mar 23, 2023 •

edited by YunFeng0817

Loading

changeset-bot bot commented Mar 23, 2023 •

edited

Loading

wfk007 commented Apr 20, 2023 •

edited

Loading

wfk007 commented Apr 28, 2023 •

edited

Loading