Skip to content

Commit

Permalink
improve rrdom performance (#1127)
Browse files Browse the repository at this point in the history
* add more check to rrdom to make diff algorithm more robust

* fix: selector match in iframe is case-insensitive

add try catch to some fragile points

* test: increase timeout value for Jest

* improve code style

* fix: failed to execute insertBefore on Node in the diff function

this happens when ids of doctype or html element are changed in the virtual dom

also improve the code quality

* refactor diff function to make the code cleaner

* fix: virtual nodes are passed to plugin's onBuild function

* refactor the diff function and adjust the order of diff work.

* call afterAppend hook in a consistent traversal order

* improve the performance of the "contains" function

reduce the complexity from O(n) to O(logn)
a specific benchmark is needed to add further

* add a real events for benchmark

* refactor: change the data structure of childNodes from array to linked list

* remove legacy code in rrweb package

* update unit tests

* update change log
  • Loading branch information
YunFeng0817 authored Feb 14, 2023
1 parent 282c8fa commit 3cc4323
Show file tree
Hide file tree
Showing 8 changed files with 364 additions and 353 deletions.
10 changes: 10 additions & 0 deletions .changeset/serious-ants-juggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'rrdom': major
'rrdom-nodejs': major
'rrweb': patch
---

Refactor: Improve performance by 80% in a super large benchmark case.

1. Refactor: change the data structure of childNodes from array to linked list
2. Improve the performance of the "contains" function. New algorithm will reduce the complexity from O(n) to O(logn)
188 changes: 109 additions & 79 deletions packages/rrdom/src/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { parseCSSText, camelize, toCSSText } from './style';
export interface IRRNode {
parentElement: IRRNode | null;
parentNode: IRRNode | null;
childNodes: IRRNode[];
ownerDocument: IRRDocument;
readonly childNodes: IRRNode[];
readonly ELEMENT_NODE: number;
readonly TEXT_NODE: number;
// corresponding nodeType value of standard HTML Node
Expand All @@ -16,8 +16,11 @@ export interface IRRNode {

lastChild: IRRNode | null;

previousSibling: IRRNode | null;

nextSibling: IRRNode | null;

// If the node is a document or a doctype, textContent returns null.
textContent: string | null;

contains(node: IRRNode): boolean;
Expand Down Expand Up @@ -127,11 +130,16 @@ type ConstrainedConstructor<T = Record<string, unknown>> = new (
* This is designed as an abstract class so it should never be instantiated.
*/
export class BaseRRNode implements IRRNode {
public childNodes: IRRNode[] = [];
public parentElement: IRRNode | null = null;
public parentNode: IRRNode | null = null;
public textContent: string | null;
public ownerDocument: IRRDocument;
public firstChild: IRRNode | null = null;
public lastChild: IRRNode | null = null;
public previousSibling: IRRNode | null = null;
public nextSibling: IRRNode | null = null;

public textContent: string | null;

public readonly ELEMENT_NODE: number = NodeType.ELEMENT_NODE;
public readonly TEXT_NODE: number = NodeType.TEXT_NODE;
// corresponding nodeType value of standard HTML Node
Expand All @@ -144,26 +152,24 @@ export class BaseRRNode implements IRRNode {
//
}

public get firstChild(): IRRNode | null {
return this.childNodes[0] || null;
}

public get lastChild(): IRRNode | null {
return this.childNodes[this.childNodes.length - 1] || null;
}

public get nextSibling(): IRRNode | null {
const parentNode = this.parentNode;
if (!parentNode) return null;
const siblings = parentNode.childNodes;
const index = siblings.indexOf(this);
return siblings[index + 1] || null;
public get childNodes(): IRRNode[] {
const childNodes: IRRNode[] = [];
let childIterator: IRRNode | null = this.firstChild;
while (childIterator) {
childNodes.push(childIterator);
childIterator = childIterator.nextSibling;
}
return childNodes;
}

public contains(node: IRRNode) {
if (node === this) return true;
for (const child of this.childNodes) {
if (child.contains(node)) return true;
if (!(node instanceof BaseRRNode)) return false;
else if (node.ownerDocument !== this.ownerDocument) return false;
else if (node === this) return true;

while (node.parentNode) {
if (node.parentNode === this) return true;
node = node.parentNode;
}
return false;
}
Expand Down Expand Up @@ -202,11 +208,11 @@ export function BaseRRDocumentImpl<
public readonly nodeName: '#document' = '#document';
public readonly compatMode: 'BackCompat' | 'CSS1Compat' = 'CSS1Compat';
public readonly RRNodeType = RRNodeType.Document;
public textContent: string | null = null;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
constructor(...args: any[]) {
super(args);
this.textContent = null;
this.ownerDocument = this;
}

Expand Down Expand Up @@ -248,8 +254,8 @@ export function BaseRRDocumentImpl<
return this.documentElement;
}

public appendChild(childNode: IRRNode): IRRNode {
const nodeType = childNode.RRNodeType;
public appendChild(newChild: IRRNode): IRRNode {
const nodeType = newChild.RRNodeType;
if (
nodeType === RRNodeType.Element ||
nodeType === RRNodeType.DocumentType
Expand All @@ -262,11 +268,10 @@ export function BaseRRDocumentImpl<
);
}
}
childNode.parentElement = null;
childNode.parentNode = this;
childNode.ownerDocument = this.ownerDocument;
this.childNodes.push(childNode);
return childNode;

const child = appendChild(this, newChild);
child.parentElement = null;
return child;
}

public insertBefore(newChild: IRRNode, refChild: IRRNode | null): IRRNode {
Expand All @@ -283,33 +288,19 @@ export function BaseRRDocumentImpl<
);
}
}
if (refChild === null) return this.appendChild(newChild);
const childIndex = this.childNodes.indexOf(refChild);
if (childIndex == -1)
throw new Error(
"Failed to execute 'insertBefore' on 'RRNode': The RRNode before which the new node is to be inserted is not a child of this RRNode.",
);
this.childNodes.splice(childIndex, 0, newChild);
newChild.parentElement = null;
newChild.parentNode = this;
newChild.ownerDocument = this.ownerDocument;
return newChild;
}

public removeChild(node: IRRNode) {
const indexOfChild = this.childNodes.indexOf(node);
if (indexOfChild === -1)
throw new Error(
"Failed to execute 'removeChild' on 'RRDocument': The RRNode to be removed is not a child of this RRNode.",
);
this.childNodes.splice(indexOfChild, 1);
node.parentElement = null;
node.parentNode = null;
return node;

const child = insertBefore(this, newChild, refChild);
child.parentElement = null;
return child;
}

public removeChild(node: IRRNode): IRRNode {
return removeChild(this, node);
}

public open() {
this.childNodes = [];
this.firstChild = null;
this.lastChild = null;
}

public close() {
Expand Down Expand Up @@ -415,14 +406,14 @@ export function BaseRRDocumentTypeImpl<
public readonly name: string;
public readonly publicId: string;
public readonly systemId: string;
public textContent: string | null = null;

constructor(qualifiedName: string, publicId: string, systemId: string) {
super();
this.name = qualifiedName;
this.publicId = publicId;
this.systemId = systemId;
this.nodeName = qualifiedName;
this.textContent = null;
}

toString() {
Expand Down Expand Up @@ -459,7 +450,9 @@ export function BaseRRElementImpl<
}

public set textContent(textContent: string) {
this.childNodes = [this.ownerDocument.createTextNode(textContent)];
this.firstChild = null;
this.lastChild = null;
this.appendChild(this.ownerDocument.createTextNode(textContent));
}

public get classList(): ClassList {
Expand Down Expand Up @@ -528,37 +521,15 @@ export function BaseRRElementImpl<
}

public appendChild(newChild: IRRNode): IRRNode {
this.childNodes.push(newChild);
newChild.parentNode = this;
newChild.parentElement = this;
newChild.ownerDocument = this.ownerDocument;
return newChild;
return appendChild(this, newChild);
}

public insertBefore(newChild: IRRNode, refChild: IRRNode | null): IRRNode {
if (refChild === null) return this.appendChild(newChild);
const childIndex = this.childNodes.indexOf(refChild);
if (childIndex == -1)
throw new Error(
"Failed to execute 'insertBefore' on 'RRNode': The RRNode before which the new node is to be inserted is not a child of this RRNode.",
);
this.childNodes.splice(childIndex, 0, newChild);
newChild.parentElement = this;
newChild.parentNode = this;
newChild.ownerDocument = this.ownerDocument;
return newChild;
return insertBefore(this, newChild, refChild);
}

public removeChild(node: IRRNode): IRRNode {
const indexOfChild = this.childNodes.indexOf(node);
if (indexOfChild === -1)
throw new Error(
"Failed to execute 'removeChild' on 'RRElement': The RRNode to be removed is not a child of this RRNode.",
);
this.childNodes.splice(indexOfChild, 1);
node.parentElement = null;
node.parentNode = null;
return node;
return removeChild(this, node);
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
Expand Down Expand Up @@ -741,6 +712,65 @@ export type CSSStyleDeclaration = Record<string, string> & {
removeProperty: (name: string) => string;
};

function appendChild(parent: IRRNode, newChild: IRRNode) {
if (parent.lastChild) {
parent.lastChild.nextSibling = newChild;
newChild.previousSibling = parent.lastChild;
} else {
parent.firstChild = newChild;
newChild.previousSibling = null;
}
parent.lastChild = newChild;
newChild.nextSibling = null;
newChild.parentNode = parent;
newChild.parentElement = parent;
newChild.ownerDocument = parent.ownerDocument;
return newChild;
}

function insertBefore(
parent: IRRNode,
newChild: IRRNode,
refChild: IRRNode | null,
) {
if (!refChild) return appendChild(parent, newChild);

if (refChild.parentNode !== parent)
throw new Error(
"Failed to execute 'insertBefore' on 'RRNode': The RRNode before which the new node is to be inserted is not a child of this RRNode.",
);

newChild.previousSibling = refChild.previousSibling;
refChild.previousSibling = newChild;
newChild.nextSibling = refChild;

if (newChild.previousSibling) newChild.previousSibling.nextSibling = newChild;
else parent.firstChild = newChild;

newChild.parentElement = parent;
newChild.parentNode = parent;
newChild.ownerDocument = parent.ownerDocument;
return newChild;
}

function removeChild(parent: IRRNode, child: IRRNode) {
if (child.parentNode !== parent)
throw new Error(
"Failed to execute 'removeChild' on 'RRNode': The RRNode to be removed is not a child of this RRNode.",
);
if (child.previousSibling)
child.previousSibling.nextSibling = child.nextSibling;
else parent.firstChild = child.nextSibling;
if (child.nextSibling)
child.nextSibling.previousSibling = child.previousSibling;
else parent.lastChild = child.previousSibling;
child.previousSibling = null;
child.nextSibling = null;
child.parentElement = null;
child.parentNode = null;
return child;
}

// Enumerate nodeType value of standard HTML Node.
export enum NodeType {
PLACEHOLDER, // This isn't a node type. Enum type value starts from zero but NodeType value starts from 1.
Expand Down
3 changes: 2 additions & 1 deletion packages/rrdom/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ export class RRDocument extends BaseRRDocumentImpl(RRNode) {
}

destroyTree() {
this.childNodes = [];
this.firstChild = null;
this.lastChild = null;
this.mirror.reset();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/rrdom/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ describe('diff algorithm for rrdom', () => {
expect(rrdom.mirror.getId(rrdom)).toBe(-2);
expect(rrdom.mirror.getId(rrdom.body)).toBe(-6);

rrdom.childNodes = [];
while (rrdom.firstChild) rrdom.removeChild(rrdom.firstChild);
/**
* Rebuild the rrdom and make it looks like this:
* -7 RRDocument
Expand Down
Loading

0 comments on commit 3cc4323

Please sign in to comment.