Skip to content

Commit 4e891e4

Browse files
committed
testing: improve performance when ending test with large number of results
Takes `markTaskComplete` 11,200ms to 70ms when running a 10k test suite, by maintaining a count of computed states for children and avoiding refreshing nodes unnecessarily. For microsoft/vscode-python#21507
1 parent d21c749 commit 4e891e4

File tree

5 files changed

+87
-60
lines changed

5 files changed

+87
-60
lines changed

src/vs/workbench/contrib/testing/browser/testingProgressUiService.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import { ProgressLocation, UnmanagedProgress } from 'vs/platform/progress/common
1313
import { IViewsService } from 'vs/workbench/common/views';
1414
import { AutoOpenTesting, getTestingConfiguration, TestingConfigKeys } from 'vs/workbench/contrib/testing/common/configuration';
1515
import { Testing } from 'vs/workbench/contrib/testing/common/constants';
16-
import { isFailedState } from 'vs/workbench/contrib/testing/common/testingStates';
17-
import { LiveTestResult, TestResultItemChangeReason, TestStateCount } from 'vs/workbench/contrib/testing/common/testResult';
16+
import { isFailedState, TestStateCount } from 'vs/workbench/contrib/testing/common/testingStates';
17+
import { LiveTestResult, TestResultItemChangeReason } from 'vs/workbench/contrib/testing/common/testResult';
1818
import { ITestResultService } from 'vs/workbench/contrib/testing/common/testResultService';
1919
import { TestResultState } from 'vs/workbench/contrib/testing/common/testTypes';
2020

src/vs/workbench/contrib/testing/common/getComputedState.ts

+49-13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { Iterable } from 'vs/base/common/iterator';
77
import { TestResultState } from 'vs/workbench/contrib/testing/common/testTypes';
8-
import { maxPriority, statePriority } from 'vs/workbench/contrib/testing/common/testingStates';
8+
import { makeEmptyCounts, maxPriority, statePriority } from 'vs/workbench/contrib/testing/common/testingStates';
99

1010
/**
1111
* Accessor for nodes in get and refresh computed state.
@@ -32,18 +32,28 @@ const isDurationAccessor = <T>(accessor: IComputedStateAccessor<T>): accessor is
3232
* if it was previously set.
3333
*/
3434

35-
const getComputedState = <T>(accessor: IComputedStateAccessor<T>, node: T, force = false) => {
35+
const getComputedState = <T extends object>(accessor: IComputedStateAccessor<T>, node: T, force = false) => {
3636
let computed = accessor.getCurrentComputedState(node);
3737
if (computed === undefined || force) {
3838
computed = accessor.getOwnState(node) ?? TestResultState.Unset;
3939

40+
let childrenCount = 0;
41+
const stateMap = makeEmptyCounts();
42+
4043
for (const child of accessor.getChildren(node)) {
4144
const childComputed = getComputedState(accessor, child);
45+
childrenCount++;
46+
stateMap[childComputed]++;
47+
4248
// If all children are skipped, make the current state skipped too if unset (#131537)
4349
computed = childComputed === TestResultState.Skipped && computed === TestResultState.Unset
4450
? TestResultState.Skipped : maxPriority(computed, childComputed);
4551
}
4652

53+
if (childrenCount > LARGE_NODE_THRESHOLD) {
54+
largeNodeChildrenStates.set(node, stateMap);
55+
}
56+
4757
accessor.setComputedState(node, computed);
4858
}
4959

@@ -72,11 +82,19 @@ const getComputedDuration = <T>(accessor: IComputedStateAndDurationAccessor<T>,
7282
return computed;
7383
};
7484

85+
const LARGE_NODE_THRESHOLD = 64;
86+
87+
/**
88+
* Map of how many nodes have in each state. This is used to optimize state
89+
* computation in large nodes with children above the `LARGE_NODE_THRESHOLD`.
90+
*/
91+
const largeNodeChildrenStates = new WeakMap<object, { [K in TestResultState]: number }>();
92+
7593
/**
7694
* Refreshes the computed state for the node and its parents. Any changes
7795
* elements cause `addUpdated` to be called.
7896
*/
79-
export const refreshComputedState = <T>(
97+
export const refreshComputedState = <T extends object>(
8098
accessor: IComputedStateAccessor<T>,
8199
node: T,
82100
explicitNewComputedState?: TestResultState,
@@ -92,28 +110,46 @@ export const refreshComputedState = <T>(
92110
accessor.setComputedState(node, newState);
93111
toUpdate.add(node);
94112

95-
if (newPriority > oldPriority) {
96-
// Update all parents to ensure they're at least this priority.
97-
for (const parent of accessor.getParents(node)) {
98-
const prev = accessor.getCurrentComputedState(parent);
113+
let moveFromState = oldState;
114+
let moveToState = newState;
115+
116+
for (const parent of accessor.getParents(node)) {
117+
const lnm = largeNodeChildrenStates.get(parent);
118+
if (lnm) {
119+
lnm[moveFromState]--;
120+
lnm[moveToState]++;
121+
}
122+
123+
const prev = accessor.getCurrentComputedState(parent);
124+
if (newPriority > oldPriority) {
125+
// Update all parents to ensure they're at least this priority.
99126
if (prev !== undefined && statePriority[prev] >= newPriority) {
100127
break;
101128
}
102129

130+
if (lnm && lnm[moveToState] > 0) {
131+
break;
132+
}
133+
134+
// moveToState remains the same, the new higher priority node state
103135
accessor.setComputedState(parent, newState);
104136
toUpdate.add(parent);
105-
}
106-
} else if (newPriority < oldPriority) {
107-
// Re-render all parents of this node whose computed priority might have come from this node
108-
for (const parent of accessor.getParents(node)) {
109-
const prev = accessor.getCurrentComputedState(parent);
137+
} else /* newProirity < oldPriority */ {
138+
// Update all parts whose statese might have been based on this one
110139
if (prev === undefined || statePriority[prev] > oldPriority) {
111140
break;
112141
}
113142

114-
accessor.setComputedState(parent, getComputedState(accessor, parent, true));
143+
if (lnm && lnm[moveFromState] > 0) {
144+
break;
145+
}
146+
147+
moveToState = getComputedState(accessor, parent, true);
148+
accessor.setComputedState(parent, moveToState);
115149
toUpdate.add(parent);
116150
}
151+
152+
moveFromState = prev;
117153
}
118154
}
119155

src/vs/workbench/contrib/testing/common/testResult.ts

+2-16
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { IComputedStateAccessor, refreshComputedState } from 'vs/workbench/contr
1414
import { IObservableValue, MutableObservableValue, staticObservableValue } from 'vs/workbench/contrib/testing/common/observableValue';
1515
import { TestCoverage } from 'vs/workbench/contrib/testing/common/testCoverage';
1616
import { TestId } from 'vs/workbench/contrib/testing/common/testId';
17-
import { maxPriority, statesInOrder, terminalStatePriorities } from 'vs/workbench/contrib/testing/common/testingStates';
17+
import { makeEmptyCounts, maxPriority, statesInOrder, terminalStatePriorities, TestStateCount } from 'vs/workbench/contrib/testing/common/testingStates';
1818
import { getMarkId, IRichLocation, ISerializedTestResults, ITestItem, ITestMessage, ITestOutputMessage, ITestRunTask, ITestTaskState, ResolvedTestRunRequest, TestItemExpandState, TestMessageType, TestResultItem, TestResultState } from 'vs/workbench/contrib/testing/common/testTypes';
1919

2020
export interface ITestRunTaskResults extends ITestRunTask {
@@ -185,20 +185,6 @@ export const resultItemParents = function* (results: ITestResult, item: TestResu
185185
}
186186
};
187187

188-
/**
189-
* Count of the number of tests in each run state.
190-
*/
191-
export type TestStateCount = { [K in TestResultState]: number };
192-
193-
export const makeEmptyCounts = () => {
194-
const o: Partial<TestStateCount> = {};
195-
for (const state of statesInOrder) {
196-
o[state] = 0;
197-
}
198-
199-
return o as TestStateCount;
200-
};
201-
202188
export const maxCountPriority = (counts: Readonly<TestStateCount>) => {
203189
for (const state of statesInOrder) {
204190
if (counts[state] > 0) {
@@ -266,7 +252,7 @@ export class LiveTestResult implements ITestResult {
266252
/**
267253
* @inheritdoc
268254
*/
269-
public readonly counts: { [K in TestResultState]: number } = makeEmptyCounts();
255+
public readonly counts = makeEmptyCounts();
270256

271257
/**
272258
* @inheritdoc

src/vs/workbench/contrib/testing/common/testingStates.ts

+10
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,13 @@ export const terminalStatePriorities: { [key in TestResultState]?: number } = {
6969
[TestResultState.Failed]: 2,
7070
[TestResultState.Errored]: 3,
7171
};
72+
73+
/**
74+
* Count of the number of tests in each run state.
75+
*/
76+
export type TestStateCount = { [K in TestResultState]: number };
77+
78+
export const makeEmptyCounts = (): TestStateCount => {
79+
// shh! don't tell anyone this is actually an array!
80+
return new Uint32Array(statesInOrder.length) as any as { [K in TestResultState]: number };
81+
};

src/vs/workbench/contrib/testing/test/common/testResultService.test.ts

+24-29
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKe
1010
import { NullLogService } from 'vs/platform/log/common/log';
1111
import { TestId } from 'vs/workbench/contrib/testing/common/testId';
1212
import { TestProfileService } from 'vs/workbench/contrib/testing/common/testProfileService';
13-
import { HydratedTestResult, LiveTestResult, makeEmptyCounts, resultItemParents, TaskRawOutput, TestResultItemChange, TestResultItemChangeReason } from 'vs/workbench/contrib/testing/common/testResult';
13+
import { HydratedTestResult, LiveTestResult, resultItemParents, TaskRawOutput, TestResultItemChange, TestResultItemChangeReason } from 'vs/workbench/contrib/testing/common/testResult';
1414
import { TestResultService } from 'vs/workbench/contrib/testing/common/testResultService';
1515
import { InMemoryResultStorage, ITestResultStorage } from 'vs/workbench/contrib/testing/common/testResultStorage';
1616
import { ITestTaskState, ResolvedTestRunRequest, TestResultItem, TestResultState, TestRunProfileBitset } from 'vs/workbench/contrib/testing/common/testTypes';
17+
import { makeEmptyCounts } from 'vs/workbench/contrib/testing/common/testingStates';
1718
import { getInitializedMainTestCollection, testStubs, TestTestCollection } from 'vs/workbench/contrib/testing/test/common/testStubs';
1819
import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';
1920

@@ -93,27 +94,24 @@ suite('Workbench - Test Results Service', () => {
9394
});
9495

9596
test('initializes with valid counts', () => {
96-
assert.deepStrictEqual(r.counts, {
97-
...makeEmptyCounts(),
98-
[TestResultState.Unset]: 4,
99-
});
97+
const c = makeEmptyCounts();
98+
c[TestResultState.Unset] = 4;
99+
assert.deepStrictEqual(r.counts, c);
100100
});
101101

102102
test('setAllToState', () => {
103103
changed.clear();
104104
r.setAllToStatePublic(TestResultState.Queued, 't', (_, t) => t.item.label !== 'root');
105-
assert.deepStrictEqual(r.counts, {
106-
...makeEmptyCounts(),
107-
[TestResultState.Unset]: 1,
108-
[TestResultState.Queued]: 3,
109-
});
105+
const c = makeEmptyCounts();
106+
c[TestResultState.Unset] = 1;
107+
c[TestResultState.Queued] = 3;
108+
assert.deepStrictEqual(r.counts, c);
110109

111110
r.setAllToStatePublic(TestResultState.Failed, 't', (_, t) => t.item.label !== 'root');
112-
assert.deepStrictEqual(r.counts, {
113-
...makeEmptyCounts(),
114-
[TestResultState.Unset]: 1,
115-
[TestResultState.Failed]: 3,
116-
});
111+
const c2 = makeEmptyCounts();
112+
c2[TestResultState.Unset] = 1;
113+
c2[TestResultState.Failed] = 3;
114+
assert.deepStrictEqual(r.counts, c2);
117115

118116
assert.deepStrictEqual(r.getStateById(new TestId(['ctrlId', 'id-a']).toString())?.ownComputedState, TestResultState.Failed);
119117
assert.deepStrictEqual(r.getStateById(new TestId(['ctrlId', 'id-a']).toString())?.tasks[0].state, TestResultState.Failed);
@@ -134,11 +132,10 @@ suite('Workbench - Test Results Service', () => {
134132
changed.clear();
135133
const testId = new TestId(['ctrlId', 'id-a', 'id-aa']).toString();
136134
r.updateState(testId, 't', TestResultState.Running);
137-
assert.deepStrictEqual(r.counts, {
138-
...makeEmptyCounts(),
139-
[TestResultState.Unset]: 3,
140-
[TestResultState.Running]: 1,
141-
});
135+
const c = makeEmptyCounts();
136+
c[TestResultState.Running] = 1;
137+
c[TestResultState.Unset] = 3;
138+
assert.deepStrictEqual(r.counts, c);
142139
assert.deepStrictEqual(r.getStateById(testId)?.ownComputedState, TestResultState.Running);
143140
// update computed state:
144141
assert.deepStrictEqual(r.getStateById(tests.root.id)?.computedState, TestResultState.Running);
@@ -161,10 +158,9 @@ suite('Workbench - Test Results Service', () => {
161158
test('ignores outside run', () => {
162159
changed.clear();
163160
r.updateState(new TestId(['ctrlId', 'id-b']).toString(), 't', TestResultState.Running);
164-
assert.deepStrictEqual(r.counts, {
165-
...makeEmptyCounts(),
166-
[TestResultState.Unset]: 4,
167-
});
161+
const c = makeEmptyCounts();
162+
c[TestResultState.Unset] = 4;
163+
assert.deepStrictEqual(r.counts, c);
168164
assert.deepStrictEqual(r.getStateById(new TestId(['ctrlId', 'id-b']).toString()), undefined);
169165
});
170166

@@ -175,11 +171,10 @@ suite('Workbench - Test Results Service', () => {
175171

176172
r.markComplete();
177173

178-
assert.deepStrictEqual(r.counts, {
179-
...makeEmptyCounts(),
180-
[TestResultState.Passed]: 1,
181-
[TestResultState.Unset]: 3,
182-
});
174+
const c = makeEmptyCounts();
175+
c[TestResultState.Unset] = 3;
176+
c[TestResultState.Passed] = 1;
177+
assert.deepStrictEqual(r.counts, c);
183178

184179
assert.deepStrictEqual(r.getStateById(tests.root.id)?.ownComputedState, TestResultState.Unset);
185180
assert.deepStrictEqual(r.getStateById(new TestId(['ctrlId', 'id-a', 'id-aa']).toString())?.ownComputedState, TestResultState.Passed);

0 commit comments

Comments
 (0)