From d7b34108dd7abcc5181f72d4276151cf1c421280 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Fri, 10 Feb 2023 16:36:27 +0100 Subject: [PATCH] Support pushing large number of items in tree iterators - Provide utility to push in a callstack-safe manner - Add tests Fixes https://github.com/eclipse-theia/theia/issues/12171 --- .../core/src/browser/tree/tree-iterator.ts | 13 +++---- .../browser/tree/tree-selection-state.spec.ts | 33 +++++++++++++++-- packages/core/src/common/array-utils.spec.ts | 36 +++++++++++++++++++ packages/core/src/common/array-utils.ts | 29 +++++++++++++++ 4 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 packages/core/src/common/array-utils.spec.ts diff --git a/packages/core/src/browser/tree/tree-iterator.ts b/packages/core/src/browser/tree/tree-iterator.ts index b09b4099521d7..89458e15fcdfe 100644 --- a/packages/core/src/browser/tree/tree-iterator.ts +++ b/packages/core/src/browser/tree/tree-iterator.ts @@ -1,5 +1,5 @@ // ***************************************************************************** -// Copyright (C) 2017 TypeFox and others. +// Copyright (C) 2017-2023 TypeFox and others. // // This program and the accompanying materials are made available under the // terms of the Eclipse Public License v. 2.0 which is available at @@ -14,7 +14,8 @@ // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 // ***************************************************************************** -import { TreeNode, CompositeTreeNode } from './tree'; +import { ArrayUtils } from '../../common'; +import { CompositeTreeNode, TreeNode } from './tree'; import { ExpandableTreeNode } from './tree-expansion'; export interface TreeIterator extends Iterator { @@ -202,12 +203,12 @@ export namespace Iterators { * Generator for depth first, pre-order tree traversal iteration. */ export function* depthFirst(root: T, children: (node: T) => T[] | undefined, include: (node: T) => boolean = () => true): IterableIterator { - const stack: T[] = []; + let stack: T[] = []; stack.push(root); while (stack.length > 0) { const top = stack.pop()!; yield top; - stack.push(...(children(top) || []).filter(include).reverse()); + stack = ArrayUtils.pushAll(stack, (children(top) || []).filter(include).reverse()); } } @@ -215,12 +216,12 @@ export namespace Iterators { * Generator for breadth first tree traversal iteration. */ export function* breadthFirst(root: T, children: (node: T) => T[] | undefined, include: (node: T) => boolean = () => true): IterableIterator { - const queue: T[] = []; + let queue: T[] = []; queue.push(root); while (queue.length > 0) { const head = queue.shift()!; yield head; - queue.push(...(children(head) || []).filter(include)); + queue = ArrayUtils.pushAll(queue, (children(head) || []).filter(include)); } } diff --git a/packages/core/src/browser/tree/tree-selection-state.spec.ts b/packages/core/src/browser/tree/tree-selection-state.spec.ts index 52ae927602433..4e5974f1ab52b 100644 --- a/packages/core/src/browser/tree/tree-selection-state.spec.ts +++ b/packages/core/src/browser/tree/tree-selection-state.spec.ts @@ -1,5 +1,5 @@ // ***************************************************************************** -// Copyright (C) 2018 TypeFox and others. +// Copyright (C) 2018-2023 TypeFox and others. // // This program and the accompanying materials are made available under the // terms of the Eclipse Public License v. 2.0 which is available at @@ -16,10 +16,10 @@ import { expect } from 'chai'; import { MockTreeModel } from './test/mock-tree-model'; -import { TreeSelectionState } from './tree-selection-state'; import { createTreeTestContainer } from './test/tree-test-container'; -import { SelectableTreeNode, TreeSelection } from './tree-selection'; import { TreeModel } from './tree-model'; +import { SelectableTreeNode, TreeSelection } from './tree-selection'; +import { TreeSelectionState } from './tree-selection-state'; namespace TreeSelectionState { @@ -34,6 +34,16 @@ namespace TreeSelectionState { } +const LARGE_FLAT_MOCK_ROOT = (length = 250000) => { + const children = Array.from({ length }, (_, idx) => ({ 'id': (idx + 1).toString() })); + return MockTreeModel.Node.toTreeNode({ + 'id': 'ROOT', + 'children': [ + ...children + ] + }); +}; + describe('tree-selection-state', () => { const model = createTreeModel(); @@ -391,6 +401,23 @@ describe('tree-selection-state', () => { }); }); + it('should be able to handle range selection on large tree', () => { + model.root = LARGE_FLAT_MOCK_ROOT(); + expect(model.selectedNodes).to.be.empty; + + const start = 10; + const end = 20; + newState() + .nextState('toggle', start.toString(), { + focus: start.toString(), + selection: [start.toString()] + }) + .nextState('range', end.toString(), { + focus: start.toString(), + selection: Array.from({ length: end - start + 1 }, (_, idx) => (start + idx).toString()) + }); + }); + function newState(): TreeSelectionState.Assert { return nextState(new TreeSelectionState(model)); } diff --git a/packages/core/src/common/array-utils.spec.ts b/packages/core/src/common/array-utils.spec.ts new file mode 100644 index 0000000000000..8fa79b689cc93 --- /dev/null +++ b/packages/core/src/common/array-utils.spec.ts @@ -0,0 +1,36 @@ +// ***************************************************************************** +// Copyright (C) 2023 EclipseSource and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 +// ***************************************************************************** + +import { expect } from 'chai'; +import { ArrayUtils } from './array-utils'; + +describe('array-utils', () => { + it('pushAll should allow pushing of large number of items', () => { + const initial = Array.from({ length: 10 }); + const addition = Array.from({ length: 1000000 }); + + const result = ArrayUtils.pushAll(initial, addition); + expect(result.length).to.equal(1000010); + }); + + it('pushAll should allow pushing of large number of items, independent from order', () => { + const initial = Array.from({ length: 1000000 }); + const addition = Array.from({ length: 10 }); + + const result = ArrayUtils.pushAll(initial, addition); + expect(result.length).to.equal(1000010); + }); +}); diff --git a/packages/core/src/common/array-utils.ts b/packages/core/src/common/array-utils.ts index 259b3822bc7c9..6fceea78a4a9a 100644 --- a/packages/core/src/common/array-utils.ts +++ b/packages/core/src/common/array-utils.ts @@ -106,4 +106,33 @@ export namespace ArrayUtils { export function coalesce(array: ReadonlyArray): T[] { return array.filter(e => !!e); } + + /** + * A safe variant to push additional items to an array. By default, the + * array push operation in combination with the spread operator is used. + * However, if the callstack size is exceeded on large additions we use the + * concatenation of arrays instead as it not depend on the callstack + * size. + * + * The original array might be modified. + * + * @param array An array of elements. + * @param items Additional elements to be added to the array. + * @returns An array containing the original elements with the additional + * elements appended. This may or may not be the array that was handed in. + */ + export function pushAll(array: T[], items: T[]): T[] { + try { + // typically faster but might fail depending on the number of items and the callstack size + array.push(...items); + return array; + } catch (error) { + if (error instanceof RangeError) { + // typically slower but works if we otherwise exceed the callstack size + // according to online benchmarks concat is faster than a forEach push + return array.concat(items); + } + throw error; + } + } }