Skip to content

Commit

Permalink
Support pushing large number of items in tree iterators
Browse files Browse the repository at this point in the history
- Provide utility to push in a callstack-safe manner
- Add tests

Fixes #12171
  • Loading branch information
martin-fleck-at committed Feb 21, 2023
1 parent 798366f commit 907a614
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 9 deletions.
13 changes: 7 additions & 6 deletions packages/core/src/browser/tree/tree-iterator.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<TreeNode> {
Expand Down Expand Up @@ -202,25 +203,25 @@ export namespace Iterators {
* Generator for depth first, pre-order tree traversal iteration.
*/
export function* depthFirst<T>(root: T, children: (node: T) => T[] | undefined, include: (node: T) => boolean = () => true): IterableIterator<T> {
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());
}
}

/**
* Generator for breadth first tree traversal iteration.
*/
export function* breadthFirst<T>(root: T, children: (node: T) => T[] | undefined, include: (node: T) => boolean = () => true): IterableIterator<T> {
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));
}
}

Expand Down
33 changes: 30 additions & 3 deletions packages/core/src/browser/tree/tree-selection-state.spec.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {

Expand All @@ -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();
Expand Down Expand Up @@ -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));
}
Expand Down
36 changes: 36 additions & 0 deletions packages/core/src/common/array-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
29 changes: 29 additions & 0 deletions packages/core/src/common/array-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,33 @@ export namespace ArrayUtils {
export function coalesce<T>(array: ReadonlyArray<T | undefined | null>): T[] {
return <T[]>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<T>(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;
}
}
}

0 comments on commit 907a614

Please sign in to comment.