From bd2082e8c574db065bb4462f30ea3ace2cb028cb Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Mon, 25 Sep 2023 14:21:29 +0200 Subject: [PATCH] Fix slow appearance of nodes on tree view The tree view rendering performance is optimized by improving the node render queue ordering. The node rendering order is modified based on the expansion state and the depth in the hierarchy, leading to faster rendering of visible nodes. This optimization is applied when the tree nodes are not expanded to improve the rendering speed. This new ordering ensures that nodes are rendered more efficiently, prioritizing nodes that are collapsed and are at a higher level in the hierarchy. --- .../TreeView/Node/HierarchicalTreeNode.vue | 2 +- .../Ordering/CollapseDepthOrderer.ts | 29 ++++++ .../Rendering/Ordering/RenderQueueOrderer.ts | 5 + .../{ => Scheduling}/NodeRenderingStrategy.ts | 2 +- .../{ => Scheduling}/TimeoutDelayScheduler.ts | 2 +- .../Rendering/UseGradualNodeRendering.ts | 22 ++--- .../View/Tree/TreeView/TreeRoot/TreeRoot.vue | 2 +- .../Ordering/CollapseDepthOrderer.spec.ts | 93 +++++++++++++++++++ .../TimeoutDelayScheduler.spec.ts | 2 +- .../Rendering/UseGradualNodeRendering.spec.ts | 48 +++++++++- .../unit/shared/Stubs/HierarchyAccessStub.ts | 4 +- .../shared/Stubs/RenderQueueOrdererStub.ts | 8 ++ .../Stubs/TreeNodeStateDescriptorStub.ts | 5 + 13 files changed, 205 insertions(+), 19 deletions(-) create mode 100644 src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.ts create mode 100644 src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer.ts rename src/presentation/components/Scripts/View/Tree/TreeView/Rendering/{ => Scheduling}/NodeRenderingStrategy.ts (63%) rename src/presentation/components/Scripts/View/Tree/TreeView/Rendering/{ => Scheduling}/TimeoutDelayScheduler.ts (94%) create mode 100644 tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.spec.ts rename tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/{ => Scheduling}/TimeoutDelayScheduler.spec.ts (97%) create mode 100644 tests/unit/shared/Stubs/RenderQueueOrdererStub.ts diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue b/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue index e6b6b2d9..7d9d8e4f 100644 --- a/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Node/HierarchicalTreeNode.vue @@ -51,7 +51,7 @@ import { } from 'vue'; import { TreeRoot } from '../TreeRoot/TreeRoot'; import { useCurrentTreeNodes } from '../UseCurrentTreeNodes'; -import { NodeRenderingStrategy } from '../Rendering/NodeRenderingStrategy'; +import { NodeRenderingStrategy } from '../Rendering/Scheduling/NodeRenderingStrategy'; import { useNodeState } from './UseNodeState'; import { TreeNode } from './TreeNode'; import LeafTreeNode from './LeafTreeNode.vue'; diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.ts b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.ts new file mode 100644 index 00000000..4ea614db --- /dev/null +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.ts @@ -0,0 +1,29 @@ +import { ReadOnlyTreeNode } from '../../Node/TreeNode'; +import { RenderQueueOrderer } from './RenderQueueOrderer'; + +export class CollapseDepthOrderer implements RenderQueueOrderer { + public orderNodes(nodes: Iterable): ReadOnlyTreeNode[] { + return orderNodes(nodes); + } +} + +function orderNodes(nodes: Iterable): ReadOnlyTreeNode[] { + return [...nodes] + .sort((a, b) => { + const [aCollapseStatus, bCollapseStatus] = [isNodeCollapsed(a), isNodeCollapsed(b)]; + if (aCollapseStatus !== bCollapseStatus) { + return (aCollapseStatus ? 1 : 0) - (bCollapseStatus ? 1 : 0); + } + return a.hierarchy.depthInTree - b.hierarchy.depthInTree; + }); +} + +function isNodeCollapsed(node: ReadOnlyTreeNode): boolean { + if (!node.state.current.isExpanded) { + return true; + } + if (node.hierarchy.parent) { + return isNodeCollapsed(node.hierarchy.parent); + } + return false; +} diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer.ts b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer.ts new file mode 100644 index 00000000..b4a2a000 --- /dev/null +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer.ts @@ -0,0 +1,5 @@ +import { ReadOnlyTreeNode } from '../../Node/TreeNode'; + +export interface RenderQueueOrderer { + orderNodes(nodes: Iterable): ReadOnlyTreeNode[]; +} diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/NodeRenderingStrategy.ts b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/NodeRenderingStrategy.ts similarity index 63% rename from src/presentation/components/Scripts/View/Tree/TreeView/Rendering/NodeRenderingStrategy.ts rename to src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/NodeRenderingStrategy.ts index b7801986..5cf80ff0 100644 --- a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/NodeRenderingStrategy.ts +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/NodeRenderingStrategy.ts @@ -1,4 +1,4 @@ -import { TreeNode } from '../Node/TreeNode'; +import { TreeNode } from '../../Node/TreeNode'; export interface NodeRenderingStrategy { shouldRender(node: TreeNode): boolean; diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.ts b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.ts similarity index 94% rename from src/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.ts rename to src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.ts index 55ce7015..eacc3efd 100644 --- a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.ts +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.ts @@ -1,4 +1,4 @@ -import { DelayScheduler } from './DelayScheduler'; +import { DelayScheduler } from '../DelayScheduler'; export class TimeoutDelayScheduler implements DelayScheduler { private timeoutId: ReturnType | undefined = undefined; diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.ts b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.ts index 15915d4f..34661719 100644 --- a/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.ts +++ b/src/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.ts @@ -1,13 +1,15 @@ import { - WatchSource, computed, shallowRef, triggerRef, watch, + WatchSource, shallowRef, triggerRef, watch, } from 'vue'; import { ReadOnlyTreeNode } from '../Node/TreeNode'; import { useNodeStateChangeAggregator } from '../UseNodeStateChangeAggregator'; import { TreeRoot } from '../TreeRoot/TreeRoot'; import { useCurrentTreeNodes } from '../UseCurrentTreeNodes'; -import { NodeRenderingStrategy } from './NodeRenderingStrategy'; +import { NodeRenderingStrategy } from './Scheduling/NodeRenderingStrategy'; import { DelayScheduler } from './DelayScheduler'; -import { TimeoutDelayScheduler } from './TimeoutDelayScheduler'; +import { TimeoutDelayScheduler } from './Scheduling/TimeoutDelayScheduler'; +import { RenderQueueOrderer } from './Ordering/RenderQueueOrderer'; +import { CollapseDepthOrderer } from './Ordering/CollapseDepthOrderer'; /** * Renders tree nodes gradually to prevent UI freeze when loading large amounts of nodes. @@ -19,6 +21,7 @@ export function useGradualNodeRendering( scheduler: DelayScheduler = new TimeoutDelayScheduler(), initialBatchSize = 30, subsequentBatchSize = 5, + orderer: RenderQueueOrderer = new CollapseDepthOrderer(), ): NodeRenderingStrategy { const nodesToRender = new Set(); const nodesBeingRendered = shallowRef(new Set()); @@ -28,8 +31,6 @@ export function useGradualNodeRendering( const { onNodeStateChange } = useChangeAggregator(treeWatcher); const { nodes } = useTreeNodes(treeWatcher); - const orderedNodes = computed(() => nodes.value.flattenedNodes); - function updateNodeRenderQueue(node: ReadOnlyTreeNode, isVisible: boolean) { if (isVisible && !nodesToRender.has(node) @@ -47,14 +48,15 @@ export function useGradualNodeRendering( } } - watch(() => orderedNodes.value, (newNodes) => { + watch(() => nodes.value, (newNodes) => { nodesToRender.clear(); nodesBeingRendered.value.clear(); - if (!newNodes?.length) { + if (!newNodes || newNodes.flattenedNodes.length === 0) { triggerRef(nodesBeingRendered); return; } newNodes + .flattenedNodes .filter((node) => node.state.current.isVisible) .forEach((node) => nodesToRender.add(node)); beginRendering(); @@ -80,10 +82,8 @@ export function useGradualNodeRendering( return; } isRenderingInProgress = true; - const sortedNodes = Array.from(nodesToRender).sort( - (a, b) => orderedNodes.value.indexOf(a) - orderedNodes.value.indexOf(b), - ); - const currentBatch = sortedNodes.slice(0, batchSize); + const orderedNodes = orderer.orderNodes(nodesToRender); + const currentBatch = orderedNodes.slice(0, batchSize); if (currentBatch.length === 0) { return; } diff --git a/src/presentation/components/Scripts/View/Tree/TreeView/TreeRoot/TreeRoot.vue b/src/presentation/components/Scripts/View/Tree/TreeView/TreeRoot/TreeRoot.vue index 4d973672..cb878ed2 100644 --- a/src/presentation/components/Scripts/View/Tree/TreeView/TreeRoot/TreeRoot.vue +++ b/src/presentation/components/Scripts/View/Tree/TreeView/TreeRoot/TreeRoot.vue @@ -22,7 +22,7 @@ import { } from 'vue'; import HierarchicalTreeNode from '../Node/HierarchicalTreeNode.vue'; import { useCurrentTreeNodes } from '../UseCurrentTreeNodes'; -import { NodeRenderingStrategy } from '../Rendering/NodeRenderingStrategy'; +import { NodeRenderingStrategy } from '../Rendering/Scheduling/NodeRenderingStrategy'; import { TreeRoot } from './TreeRoot'; export default defineComponent({ diff --git a/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.spec.ts b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.spec.ts new file mode 100644 index 00000000..df6436e8 --- /dev/null +++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer.spec.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from 'vitest'; +import { TreeNode } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode'; +import { CollapseDepthOrderer } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/CollapseDepthOrderer'; +import { HierarchyAccessStub } from '@tests/unit/shared/Stubs/HierarchyAccessStub'; +import { TreeNodeStateAccessStub } from '@tests/unit/shared/Stubs/TreeNodeStateAccessStub'; +import { TreeNodeStateDescriptorStub } from '@tests/unit/shared/Stubs/TreeNodeStateDescriptorStub'; +import { TreeNodeStub } from '@tests/unit/shared/Stubs/TreeNodeStub'; + +describe('CollapseDepthOrderer', () => { + describe('orderNodes', () => { + it('should order by collapsed state and then by depth in', () => { + // arrange + const node1 = createNodeForOrder({ + isExpanded: false, + depthInTree: 1, + }); + const node2 = createNodeForOrder({ + isExpanded: true, + depthInTree: 2, + }); + const node3 = createNodeForOrder({ + isExpanded: false, + depthInTree: 3, + }); + const node4 = createNodeForOrder({ + isExpanded: false, + depthInTree: 4, + }); + const nodes = [node1, node2, node3, node4]; + const expectedOrder = [node2, node1, node3, node4]; + // act + const orderer = new CollapseDepthOrderer(); + const orderedNodes = orderer.orderNodes(nodes); + // assert + expect(orderedNodes.map((node) => node.id)).to.deep + .equal(expectedOrder.map((node) => node.id)); + }); + it('should handle parent collapsed state', () => { + // arrange + const collapsedParent = createNodeForOrder({ + isExpanded: false, + depthInTree: 0, + }); + const childWithCollapsedParent = createNodeForOrder({ + isExpanded: true, + depthInTree: 1, + parent: collapsedParent, + }); + const deepExpandedNode = createNodeForOrder({ + isExpanded: true, + depthInTree: 3, + }); + const nodes = [childWithCollapsedParent, collapsedParent, deepExpandedNode]; + const expectedOrder = [ + deepExpandedNode, // comes first due to collapse parent of child + collapsedParent, + childWithCollapsedParent, + ]; + // act + const orderer = new CollapseDepthOrderer(); + const orderedNodes = orderer.orderNodes(nodes); + // assert + expect(orderedNodes.map((node) => node.id)).to.deep + .equal(expectedOrder.map((node) => node.id)); + }); + }); +}); + +function createNodeForOrder(options: { + readonly isExpanded: boolean; + readonly depthInTree: number; + readonly parent?: TreeNode; +}): TreeNode { + return new TreeNodeStub() + .withId([ + `isExpanded: ${options.isExpanded}`, + `depthInTree: ${options.depthInTree}`, + ...(options.parent ? [`parent: ${options.parent.id}`] : []), + ].join(', ')) + .withState( + new TreeNodeStateAccessStub() + .withCurrent( + new TreeNodeStateDescriptorStub() + .withVisibility(true) + .withExpansion(options.isExpanded), + ), + ) + .withHierarchy( + new HierarchyAccessStub() + .withDepthInTree(options.depthInTree) + .withParent(options.parent), + ); +} diff --git a/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.spec.ts b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.spec.ts similarity index 97% rename from tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.spec.ts rename to tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.spec.ts index 5359c6a2..6e38ebb3 100644 --- a/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.spec.ts +++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { TimeFunctions, TimeoutDelayScheduler } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler'; +import { TimeFunctions, TimeoutDelayScheduler } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/Scheduling/TimeoutDelayScheduler'; import { StubWithObservableMethodCalls } from '@tests/unit/shared/Stubs/StubWithObservableMethodCalls'; describe('TimeoutDelayScheduler', () => { diff --git a/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.spec.ts b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.spec.ts index 5c6be644..b181b6c6 100644 --- a/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.spec.ts +++ b/tests/unit/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.spec.ts @@ -12,7 +12,9 @@ import { NodeStateChangeEventArgsStub } from '@tests/unit/shared/Stubs/NodeState import { TreeNodeStateDescriptorStub } from '@tests/unit/shared/Stubs/TreeNodeStateDescriptorStub'; import { DelaySchedulerStub } from '@tests/unit/shared/Stubs/DelaySchedulerStub'; import { DelayScheduler } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/DelayScheduler'; -import { TreeNode } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode'; +import { ReadOnlyTreeNode, TreeNode } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode'; +import { RenderQueueOrdererStub } from '@tests/unit/shared/Stubs/RenderQueueOrdererStub'; +import { RenderQueueOrderer } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer'; describe('useGradualNodeRendering', () => { it('watches nodes on specified tree', () => { @@ -194,6 +196,42 @@ describe('useGradualNodeRendering', () => { }); }); }); + it('orders nodes before rendering', async () => { + // arrange + const delaySchedulerStub = new DelaySchedulerStub(); + const allNodes = Array.from({ length: 3 }).map(() => createNodeWithVisibility(true)); + const expectedNodes = [ + /* initial render */ [allNodes[2]], + /* first subsequent render */ [allNodes[1]], + /* second subsequent render */ [allNodes[0]], + ]; + const ordererStub = new RenderQueueOrdererStub(); + const nodesStub = new UseCurrentTreeNodesStub().withQueryableNodes( + new QueryableNodesStub().withFlattenedNodes(allNodes), + ); + const builder = new UseGradualNodeRenderingBuilder() + .withCurrentTreeNodes(nodesStub) + .withInitialBatchSize(1) + .withSubsequentBatchSize(1) + .withDelayScheduler(delaySchedulerStub) + .withOrderer(ordererStub); + const actualOrder = new Set(); + // act + ordererStub.orderNodes = () => expectedNodes[0]; + const strategy = builder.call(); + const updateOrder = () => allNodes + .filter((node) => strategy.shouldRender(node)) + .forEach((node) => actualOrder.add(node)); + updateOrder(); + for (let i = 1; i < expectedNodes.length; i++) { + ordererStub.orderNodes = () => expectedNodes[i]; + delaySchedulerStub.runNextScheduled(); + updateOrder(); + } + // assert + const expectedOrder = expectedNodes.flat(); + expect([...actualOrder]).to.deep.equal(expectedOrder); + }); }); it('skips scheduling when no nodes to render', () => { // arrange @@ -241,6 +279,8 @@ class UseGradualNodeRenderingBuilder { private subsequentBatchSize = 3; + private orderer: RenderQueueOrderer = new RenderQueueOrdererStub(); + public withChangeAggregator(changeAggregator: UseNodeStateChangeAggregatorStub): this { this.changeAggregator = changeAggregator; return this; @@ -271,6 +311,11 @@ class UseGradualNodeRenderingBuilder { return this; } + public withOrderer(orderer: RenderQueueOrderer) { + this.orderer = orderer; + return this; + } + public call(): ReturnType { return useGradualNodeRendering( this.treeWatcher, @@ -279,6 +324,7 @@ class UseGradualNodeRenderingBuilder { this.delayScheduler, this.initialBatchSize, this.subsequentBatchSize, + this.orderer, ); } } diff --git a/tests/unit/shared/Stubs/HierarchyAccessStub.ts b/tests/unit/shared/Stubs/HierarchyAccessStub.ts index 093d524c..7fb4f6fb 100644 --- a/tests/unit/shared/Stubs/HierarchyAccessStub.ts +++ b/tests/unit/shared/Stubs/HierarchyAccessStub.ts @@ -2,7 +2,7 @@ import { HierarchyAccess } from '@/presentation/components/Scripts/View/Tree/Tre import { TreeNode } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode'; export class HierarchyAccessStub implements HierarchyAccess { - public parent: TreeNode = undefined; + public parent: TreeNode | undefined = undefined; public children: readonly TreeNode[] = []; @@ -20,7 +20,7 @@ export class HierarchyAccessStub implements HierarchyAccess { this.children = children; } - public withParent(parent: TreeNode): this { + public withParent(parent: TreeNode | undefined): this { this.parent = parent; return this; } diff --git a/tests/unit/shared/Stubs/RenderQueueOrdererStub.ts b/tests/unit/shared/Stubs/RenderQueueOrdererStub.ts new file mode 100644 index 00000000..b95de3ab --- /dev/null +++ b/tests/unit/shared/Stubs/RenderQueueOrdererStub.ts @@ -0,0 +1,8 @@ +import { ReadOnlyTreeNode } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode'; +import { RenderQueueOrderer } from '@/presentation/components/Scripts/View/Tree/TreeView/Rendering/Ordering/RenderQueueOrderer'; + +export class RenderQueueOrdererStub implements RenderQueueOrderer { + public orderNodes(nodes: Iterable): ReadOnlyTreeNode[] { + return [...nodes]; + } +} diff --git a/tests/unit/shared/Stubs/TreeNodeStateDescriptorStub.ts b/tests/unit/shared/Stubs/TreeNodeStateDescriptorStub.ts index 340a65a0..398d9ee6 100644 --- a/tests/unit/shared/Stubs/TreeNodeStateDescriptorStub.ts +++ b/tests/unit/shared/Stubs/TreeNodeStateDescriptorStub.ts @@ -26,4 +26,9 @@ export class TreeNodeStateDescriptorStub implements TreeNodeStateDescriptor { this.isVisible = isVisible; return this; } + + public withExpansion(isExpanded: boolean): this { + this.isExpanded = isExpanded; + return this; + } }