From 0e8f72ad714daaa202dc0ac32111ca205466aba2 Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Fri, 24 Jun 2022 15:11:24 +0200 Subject: [PATCH 1/6] feat(tracker): faster work with infinite lists --- .../tracker/src/main/app/observer/observer.ts | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/tracker/tracker/src/main/app/observer/observer.ts b/tracker/tracker/src/main/app/observer/observer.ts index 2283fac56..e01897a10 100644 --- a/tracker/tracker/src/main/app/observer/observer.ts +++ b/tracker/tracker/src/main/app/observer/observer.ts @@ -51,23 +51,25 @@ function isObservable(node: Node): boolean { export default abstract class Observer { private readonly observer: MutationObserver; private readonly commited: Array = []; - private readonly recents: Array = []; - private readonly myNodes: Array = []; private readonly indexes: Array = []; private readonly attributesList: Array | undefined> = []; - private readonly textSet: Set = new Set(); + private readonly textSet: Set = new Set() + private readonly newSet: Set = new Set() + private readonly affectedSet: Set = new Set() constructor(protected readonly app: App, protected readonly isTopContext = false) { this.observer = new MutationObserver( this.app.safe((mutations) => { - for (const mutation of mutations) { // mutations order is sequential + for (const mutation of mutations) { // mutations order is sequential const target = mutation.target; const type = mutation.type; - if (!isObservable(target)) { continue; } if (type === 'childList') { for (let i = 0; i < mutation.removedNodes.length; i++) { + // TODO: handle node removal separately from binding. + // Node removals should go first in the commit. + // To check: MoveNode and other possible unbinding behaviours this.bindTree(mutation.removedNodes[i]); } for (let i = 0; i < mutation.addedNodes.length; i++) { @@ -79,9 +81,6 @@ export default abstract class Observer { if (id === undefined) { continue; } - if (id >= this.recents.length) { // TODO: something more convinient - this.recents[id] = undefined; - } if (type === 'attributes') { const name = mutation.attributeName; if (name === null) { @@ -92,10 +91,12 @@ export default abstract class Observer { this.attributesList[id] = attr = new Set(); } attr.add(name); + this.affectedSet.add(id) continue; } if (type === 'characterData') { this.textSet.add(id); + this.affectedSet.add(id) continue; } } @@ -105,10 +106,11 @@ export default abstract class Observer { } private clear(): void { this.commited.length = 0; - this.recents.length = 0; this.indexes.length = 1; this.attributesList.length = 0; - this.textSet.clear(); + this.textSet.clear() + this.newSet.clear() + this.affectedSet.clear() } private sendNodeAttribute( @@ -176,11 +178,11 @@ export default abstract class Observer { } private bindNode(node: Node): void { - const r = this.app.nodes.registerNode(node); - const id = r[0]; - this.recents[id] = r[1] || this.recents[id] || false; - - this.myNodes[id] = true; + const [ id, isNew ] = this.app.nodes.registerNode(node); + if (isNew) { + this.newSet.add(id) + } + this.affectedSet.add(id) } private bindTree(node: Node): void { @@ -207,7 +209,8 @@ export default abstract class Observer { private unbindNode(node: Node): void { const id = this.app.nodes.unregisterNode(node); - if (id !== undefined && this.recents[id] === false) { +// if (id !== undefined && this.recents[id] === false) { // In the old version it === flase when bindNode() was called on node but it was not new + if (id !== undefined && !this.newSet.has(id) && this.affectedSet.has(id)) { // Unbinding logic should be simplified. Node removals should go first. this.app.send(new RemoveNode(id)); } } @@ -256,7 +259,7 @@ export default abstract class Observer { if (sibling === null) { this.indexes[id] = 0; } - const isNew = this.recents[id]; + const isNew = this.newSet.has(id) const index = this.indexes[id]; if (index === undefined) { throw 'commitNode: missing node index'; @@ -328,17 +331,12 @@ export default abstract class Observer { } private commitNodes(): void { let node; - for (let id = 0; id < this.recents.length; id++) { - // TODO: make things/logic nice here. - // commit required in any case if recents[id] true or false (in case of unbinding) or undefined (in case of attr change). - // Possible solution: separate new node commit (recents) and new attribute/move node commit - // Otherwise commitNode is called on each node, which might be a lot - if (!this.myNodes[id]) { continue } + this.affectedSet.forEach(id => { this.commitNode(id); - if (this.recents[id] === true && (node = this.app.nodes.getNode(id))) { + if (this.newSet.has(id) && (node = this.app.nodes.getNode(id))) { this.app.nodes.callNodeCallbacks(node); } - } + }) this.clear(); } @@ -360,6 +358,5 @@ export default abstract class Observer { disconnect(): void { this.observer.disconnect(); this.clear(); - this.myNodes.length = 0; } } From 2c88aece5608ebca108df41129ab3937c949246d Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 27 Jun 2022 20:33:24 +0200 Subject: [PATCH 2/6] fix(tracker): observer - use Map for recents instead of array --- .../tracker/src/main/app/observer/observer.ts | 64 ++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/tracker/tracker/src/main/app/observer/observer.ts b/tracker/tracker/src/main/app/observer/observer.ts index e01897a10..c6ede00ce 100644 --- a/tracker/tracker/src/main/app/observer/observer.ts +++ b/tracker/tracker/src/main/app/observer/observer.ts @@ -45,31 +45,34 @@ function isObservable(node: Node): boolean { /* TODO: - fix unbinding logic + send all removals first (ensure sequence is correct) - - use document as a 0-node in the upper context + - use document as a 0-node in the upper context (should be updated in player at first) */ +enum RecentsType { + New, + Removed, + Changed, +} + export default abstract class Observer { private readonly observer: MutationObserver; private readonly commited: Array = []; + private readonly recents: Map = new Map() private readonly indexes: Array = []; - private readonly attributesList: Array | undefined> = []; - private readonly textSet: Set = new Set() - private readonly newSet: Set = new Set() - private readonly affectedSet: Set = new Set() + private readonly attributesMap: Map> = new Map(); + private readonly textSet: Set = new Set(); constructor(protected readonly app: App, protected readonly isTopContext = false) { this.observer = new MutationObserver( this.app.safe((mutations) => { - for (const mutation of mutations) { // mutations order is sequential + for (const mutation of mutations) { // mutations order is sequential const target = mutation.target; const type = mutation.type; + if (!isObservable(target)) { continue; } if (type === 'childList') { for (let i = 0; i < mutation.removedNodes.length; i++) { - // TODO: handle node removal separately from binding. - // Node removals should go first in the commit. - // To check: MoveNode and other possible unbinding behaviours this.bindTree(mutation.removedNodes[i]); } for (let i = 0; i < mutation.addedNodes.length; i++) { @@ -81,22 +84,23 @@ export default abstract class Observer { if (id === undefined) { continue; } + if (!this.recents.has(id)) { + this.recents.set(id, RecentsType.Changed) // TODO only when altered + } if (type === 'attributes') { const name = mutation.attributeName; if (name === null) { continue; } - let attr = this.attributesList[id]; + let attr = this.attributesMap.get(id) if (attr === undefined) { - this.attributesList[id] = attr = new Set(); + this.attributesMap.set(id, attr = new Set()) } attr.add(name); - this.affectedSet.add(id) continue; } if (type === 'characterData') { this.textSet.add(id); - this.affectedSet.add(id) continue; } } @@ -106,11 +110,10 @@ export default abstract class Observer { } private clear(): void { this.commited.length = 0; + this.recents.clear() this.indexes.length = 1; - this.attributesList.length = 0; - this.textSet.clear() - this.newSet.clear() - this.affectedSet.clear() + this.attributesMap.clear(); + this.textSet.clear(); } private sendNodeAttribute( @@ -178,11 +181,12 @@ export default abstract class Observer { } private bindNode(node: Node): void { - const [ id, isNew ] = this.app.nodes.registerNode(node); - if (isNew) { - this.newSet.add(id) + const [ id, isNew ]= this.app.nodes.registerNode(node); + if (isNew){ + this.recents.set(id, RecentsType.New) + } else if (!this.recents.has(id)) { + this.recents.set(id, RecentsType.Removed) } - this.affectedSet.add(id) } private bindTree(node: Node): void { @@ -209,8 +213,7 @@ export default abstract class Observer { private unbindNode(node: Node): void { const id = this.app.nodes.unregisterNode(node); -// if (id !== undefined && this.recents[id] === false) { // In the old version it === flase when bindNode() was called on node but it was not new - if (id !== undefined && !this.newSet.has(id) && this.affectedSet.has(id)) { // Unbinding logic should be simplified. Node removals should go first. + if (id !== undefined && this.recents.get(id) === RecentsType.Removed) { this.app.send(new RemoveNode(id)); } } @@ -259,12 +262,13 @@ export default abstract class Observer { if (sibling === null) { this.indexes[id] = 0; } - const isNew = this.newSet.has(id) - const index = this.indexes[id]; + const recentsType = this.recents.get(id) + const isNew = recentsType === RecentsType.New + const index = this.indexes[id] if (index === undefined) { throw 'commitNode: missing node index'; } - if (isNew === true) { + if (isNew) { if (isElementNode(node)) { let el: Element = node if (parentID !== undefined) { @@ -297,10 +301,10 @@ export default abstract class Observer { } return true; } - if (isNew === false && parentID !== undefined) { + if (recentsType === RecentsType.Removed && parentID !== undefined) { this.app.send(new MoveNode(id, parentID, index)); } - const attr = this.attributesList[id]; + const attr = this.attributesMap.get(id); if (attr !== undefined) { if (!isElementNode(node)) { throw 'commitNode: node is not an element'; @@ -331,9 +335,9 @@ export default abstract class Observer { } private commitNodes(): void { let node; - this.affectedSet.forEach(id => { + this.recents.forEach((type, id) => { this.commitNode(id); - if (this.newSet.has(id) && (node = this.app.nodes.getNode(id))) { + if (type === RecentsType.New && (node = this.app.nodes.getNode(id))) { this.app.nodes.callNodeCallbacks(node); } }) From f78da2756295c589333d3419675192c0889aef07 Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 27 Jun 2022 20:34:32 +0200 Subject: [PATCH 3/6] fix(tracker): force sending on body mouseleave (works on firefox unlike document's mouseleave) --- tracker/tracker/src/main/app/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracker/tracker/src/main/app/index.ts b/tracker/tracker/src/main/app/index.ts index 25ef0a11c..4f539092c 100644 --- a/tracker/tracker/src/main/app/index.ts +++ b/tracker/tracker/src/main/app/index.ts @@ -163,7 +163,7 @@ export default class App { } // TODO: keep better tactics, discard others (look https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon) this.attachEventListener(window, 'beforeunload', alertWorker, false); - this.attachEventListener(document, 'mouseleave', alertWorker, false, false); + this.attachEventListener(document.body, 'mouseleave', alertWorker, false, false); this.attachEventListener(document, 'visibilitychange', alertWorker, false); } catch (e) { this._debug("worker_start", e); From cea704e1eed86d3d65375e59eb497f629473d285 Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Wed, 29 Jun 2022 11:10:52 +0200 Subject: [PATCH 4/6] fix(tracker): check node scrolls only on start --- tracker/tracker/src/main/app/nodes.ts | 6 +++--- tracker/tracker/src/main/app/observer/observer.ts | 6 +++--- tracker/tracker/src/main/modules/scroll.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tracker/tracker/src/main/app/nodes.ts b/tracker/tracker/src/main/app/nodes.ts index 73ba4f311..1651c7fe5 100644 --- a/tracker/tracker/src/main/app/nodes.ts +++ b/tracker/tracker/src/main/app/nodes.ts @@ -1,4 +1,4 @@ -type NodeCallback = (node: Node) => void; +type NodeCallback = (node: Node, isStart: boolean) => void; type ElementListener = [string, EventListener]; export default class Nodes { @@ -57,8 +57,8 @@ export default class Nodes { } return id; } - callNodeCallbacks(node: Node): void { - this.nodeCallbacks.forEach((cb) => cb(node)); + callNodeCallbacks(node: Node, start: boolean): void { + this.nodeCallbacks.forEach((cb) => cb(node, start)); } getID(node: Node): number | undefined { return (node as any)[this.node_id]; diff --git a/tracker/tracker/src/main/app/observer/observer.ts b/tracker/tracker/src/main/app/observer/observer.ts index c6ede00ce..dfaaa3986 100644 --- a/tracker/tracker/src/main/app/observer/observer.ts +++ b/tracker/tracker/src/main/app/observer/observer.ts @@ -333,12 +333,12 @@ export default abstract class Observer { } return (this.commited[id] = this._commitNode(id, node)); } - private commitNodes(): void { + private commitNodes(isStart: boolean = false): void { let node; this.recents.forEach((type, id) => { this.commitNode(id); if (type === RecentsType.New && (node = this.app.nodes.getNode(id))) { - this.app.nodes.callNodeCallbacks(node); + this.app.nodes.callNodeCallbacks(node, isStart) } }) this.clear(); @@ -356,7 +356,7 @@ export default abstract class Observer { }); this.bindTree(nodeToBind); beforeCommit(this.app.nodes.getID(node)) - this.commitNodes(); + this.commitNodes(true) } disconnect(): void { diff --git a/tracker/tracker/src/main/modules/scroll.ts b/tracker/tracker/src/main/modules/scroll.ts index a16bb166a..3515b58e6 100644 --- a/tracker/tracker/src/main/modules/scroll.ts +++ b/tracker/tracker/src/main/modules/scroll.ts @@ -35,8 +35,8 @@ export default function (app: App): void { nodeScroll.clear(); }); - app.nodes.attachNodeCallback(node => { - if (isElementNode(node) && node.scrollLeft + node.scrollTop > 0) { + app.nodes.attachNodeCallback((node, isStart) => { + if (isStart && isElementNode(node) && node.scrollLeft + node.scrollTop > 0) { nodeScroll.set(node, [node.scrollLeft, node.scrollTop]); } }) From 8ad5d869f723f28329d93ece260e1ff5368bfd34 Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Wed, 29 Jun 2022 11:13:53 +0200 Subject: [PATCH 5/6] fixup! fix(tracker): check node scrolls only on start --- tracker/tracker/src/main/app/nodes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracker/tracker/src/main/app/nodes.ts b/tracker/tracker/src/main/app/nodes.ts index 1651c7fe5..a1c87b6e2 100644 --- a/tracker/tracker/src/main/app/nodes.ts +++ b/tracker/tracker/src/main/app/nodes.ts @@ -57,8 +57,8 @@ export default class Nodes { } return id; } - callNodeCallbacks(node: Node, start: boolean): void { - this.nodeCallbacks.forEach((cb) => cb(node, start)); + callNodeCallbacks(node: Node, isStart: boolean): void { + this.nodeCallbacks.forEach((cb) => cb(node, isStart)); } getID(node: Node): number | undefined { return (node as any)[this.node_id]; From cf7314260bf0cffcdf195b2d72e4b4c241838a1d Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Thu, 30 Jun 2022 17:03:55 +0200 Subject: [PATCH 6/6] feat(tracker):3.5.13: performance improvements for a case of extensive dom --- tracker/tracker/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracker/tracker/package.json b/tracker/tracker/package.json index d9e218a59..3a6c90673 100644 --- a/tracker/tracker/package.json +++ b/tracker/tracker/package.json @@ -1,7 +1,7 @@ { "name": "@openreplay/tracker", "description": "The OpenReplay tracker main package", - "version": "3.5.12", + "version": "3.5.13", "keywords": [ "logging", "replay"