Noticable performance hang when dismissing a large number of items

src/vs/workbench/contrib/search/browser/searchActions.ts

@@ -23,7 +23,7 @@ import { SearchView } from 'vs/workbench/contrib/search/browser/searchView';
 import * as Constants from 'vs/workbench/contrib/search/common/constants';
 import { IReplaceService } from 'vs/workbench/contrib/search/common/replace';
 import { ISearchHistoryService } from 'vs/workbench/contrib/search/common/searchHistoryService';
-import { FileMatch, FolderMatch, FolderMatchWithResource, Match, RenderableMatch, searchComparer, searchMatchComparer, SearchResult } from 'vs/workbench/contrib/search/common/searchModel';
+import { arrayContainsElementOrParent, FileMatch, FolderMatch, FolderMatchWithResource, Match, RenderableMatch, searchComparer, searchMatchComparer, SearchResult } from 'vs/workbench/contrib/search/common/searchModel';
 import { OpenEditorCommandId } from 'vs/workbench/contrib/searchEditor/browser/constants';
 import { SearchEditor } from 'vs/workbench/contrib/searchEditor/browser/searchEditor';
 import { OpenSearchEditorArgs } from 'vs/workbench/contrib/searchEditor/browser/searchEditor.contribution';
@@ -431,30 +431,20 @@ class ReplaceActionRunner {
 		@IReplaceService private readonly replaceService: IReplaceService,
 		@IEditorService private readonly editorService: IEditorService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
-		@IUriIdentityService private readonly uriIdentityService: IUriIdentityService
+		@IUriIdentityService private readonly uriIdentityService: IUriIdentityService,
+		@IViewsService private readonly viewsService: IViewsService
 	) { }
 
 	async performReplace(element: FolderMatch | FileMatch | Match): Promise<any> {
 		// since multiple elements can be selected, we need to check the type of the FolderMatch/FileMatch/Match before we perform the replace.
 		const opInfo = getElementsToOperateOnInfo(this.viewer, element, this.configurationService.getValue<ISearchConfigurationProperties>('search'));
 		const elementsToReplace = opInfo.elements;
 
-		await Promise.all(elementsToReplace.map(async (elem) => {
-			const parent = elem.parent();
-
-			if ((parent instanceof FolderMatch || parent instanceof FileMatch) && arrayContainsElementOrParent(parent, elementsToReplace)) {
-				// skip any children who have parents in the array
-				return;
-			}
+		const searchResult = getSearchView(this.viewsService)?.searchResult;
 
-			if (elem instanceof FileMatch) {
-				elem.parent().replace(elem);
-			} else if (elem instanceof Match) {
-				elem.parent().replace(elem);
-			} else if (elem instanceof FolderMatch) {
-				await elem.replaceAll();
-			}
-		}));
+		if (searchResult) {
+			searchResult.batchReplace(elementsToReplace);
+		}
 
 		const currentBottomFocusElement = elementsToReplace[elementsToReplace.length - 1];
 
@@ -561,6 +551,7 @@ export class RemoveAction extends AbstractSearchAndReplaceAction {
 		private element: RenderableMatch,
 		@IKeybindingService keyBindingService: IKeybindingService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@IViewsService private readonly viewsService: IViewsService,
 	) {
 		super(Constants.RemoveActionId, appendKeyBindingLabel(RemoveAction.LABEL, keyBindingService.lookupKeybinding(Constants.RemoveActionId), keyBindingService), ThemeIcon.asClassName(searchRemoveIcon));
 	}
@@ -587,24 +578,17 @@ export class RemoveAction extends AbstractSearchAndReplaceAction {
 			}
 		}
 
-		elementsToRemove.forEach((currentElement) =>
-			currentElement.parent().remove(<(FolderMatch | FileMatch)[] & Match & FileMatch[]>currentElement)
-		);
+		const searchResult = getSearchView(this.viewsService)?.searchResult;
+
+		if (searchResult) {
+			searchResult.batchRemove(elementsToRemove);
+		}
 
 		this.viewer.domFocus();
 		return;
 	}
 }
 
-function arrayContainsElementOrParent(element: RenderableMatch, testArray: (RenderableMatch | SearchResult)[]): boolean {
-	do {
-		if (testArray.includes(element)) {
-			return true;
-		}
-	} while (!(element.parent() instanceof SearchResult) && (element = <RenderableMatch>element.parent()));
-
-	return false;
-}
 
 export class ReplaceAllAction extends AbstractSearchAndReplaceAction {
 

src/vs/workbench/contrib/search/common/searchModel.ts

@@ -8,7 +8,7 @@ import { CancellationTokenSource } from 'vs/base/common/cancellation';
 import { compareFileExtensions, compareFileNames, comparePaths } from 'vs/base/common/comparers';
 import { memoize } from 'vs/base/common/decorators';
 import * as errors from 'vs/base/common/errors';
-import { Emitter, Event } from 'vs/base/common/event';
+import { Emitter, Event, PauseableEmitter } from 'vs/base/common/event';
 import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
 import { ResourceMap, TernarySearchTree } from 'vs/base/common/map';
 import { Schemas } from 'vs/base/common/network';
@@ -368,9 +368,16 @@ export class FileMatch extends Disposable implements IFileMatch {
 		return Array.from(this._matches.values());
 	}
 
-	remove(match: Match): void {
-		this.removeMatch(match);
-		this._removedMatches.add(match.id());
+	remove(matches: Match | Match[]): void {
+		if (!Array.isArray(matches)) {
+			matches = [matches];
+		}
+
+		for (const match of matches) {
+			this.removeMatch(match);
+			this._removedMatches.add(match.id());
+		}
+
 		this._onChange.fire({ didRemove: true });
 	}
 
@@ -580,7 +587,7 @@ export class FolderMatch extends Disposable {
 
 	replaceAll(): Promise<any> {
 		const matches = this.matches();
-		return this.replaceService.replace(matches).then(() => this.doRemove(matches));
+		return this.batchReplace(matches);
 	}
 
 	matches(): FileMatch[] {
@@ -599,6 +606,11 @@ export class FolderMatch extends Disposable {
 		return this.matches().reduce<number>((prev, match) => prev + match.count(), 0);
 	}
 
+	private async batchReplace(matches: FileMatch[]): Promise<any> {
+		await this.replaceService.replace(matches);
+		this.doRemove(matches, true, true);
+	}
+
 	private onFileChange(fileMatch: FileMatch, removed = false): void {
 		let added = false;
 		if (!this._fileMatches.has(fileMatch.resource)) {
@@ -749,9 +761,10 @@ function createParentList(element: RenderableMatch): RenderableMatch[] {
 }
 export class SearchResult extends Disposable {
 
-	private _onChange = this._register(new Emitter<IChangeEvent>());
+	private _onChange = this._register(new PauseableEmitter<IChangeEvent>({
+		merge: this.mergeEvents
+	}));
 	readonly onChange: Event<IChangeEvent> = this._onChange.event;
-
 	private _folderMatches: FolderMatchWithResource[] = [];
 	private _otherFilesMatch: FolderMatch | null = null;
 	private _folderMatchesMap: TernarySearchTree<URI, FolderMatchWithResource> = TernarySearchTree.forUris<FolderMatchWithResource>(key => this.uriIdentityService.extUri.ignorePathCasing(key));
@@ -782,6 +795,41 @@ export class SearchResult extends Disposable {
 		}));
 	}
 
+	async batchReplace(elementsToReplace: RenderableMatch[]) {
+		try {
+			this._onChange.pause();
+			await Promise.all(elementsToReplace.map(async (elem) => {
+				const parent = elem.parent();
+
+				if ((parent instanceof FolderMatch || parent instanceof FileMatch) && arrayContainsElementOrParent(parent, elementsToReplace)) {
+					// skip any children who have parents in the array
+					return;
+				}
+
+				if (elem instanceof FileMatch) {
+					await elem.parent().replace(elem);
+				} else if (elem instanceof Match) {
+					await elem.parent().replace(elem);
+				} else if (elem instanceof FolderMatch) {
+					await elem.replaceAll();
+				}
+			}));
+		} finally {
+			this._onChange.resume();
+		}
+	}
+
+	batchRemove(elementsToRemove: RenderableMatch[]) {
+		try {
+			this._onChange.pause();
+			elementsToRemove.forEach((currentElement) =>
+				currentElement.parent().remove(<(FolderMatch | FileMatch)[] & Match & FileMatch[]>currentElement)
+			);
+		} finally {
+			this._onChange.resume();
+		}
+	}
+
 	get isDirty(): boolean {
 		return this._isDirty;
 	}
@@ -815,6 +863,26 @@ export class SearchResult extends Disposable {
 		this._query = query;
 	}
 
+	private mergeEvents(events: IChangeEvent[]): IChangeEvent {
+		const retEvent: IChangeEvent = {
+			elements: [],
+			added: false,
+			removed: false,
+		};
+		events.forEach((e) => {
+			if (e.added) {
+				retEvent.added = true;
+			}
+
+			if (e.removed) {
+				retEvent.removed = true;
+			}
+
+			retEvent.elements = retEvent.elements.concat(e.elements);
+		});
+
+		return retEvent;
+	}
 	private onModelAdded(model: ITextModel): void {
 		const folderMatch = this._folderMatchesMap.findSubstr(model.uri);
 		folderMatch?.bindModel(model);
@@ -1350,3 +1418,13 @@ function textSearchResultToMatches(rawMatch: ITextSearchMatch, fileMatch: FileMa
 		return [match];
 	}
 }
+
+export function arrayContainsElementOrParent(element: RenderableMatch, testArray: RenderableMatch[]): boolean {
+	do {
+		if (testArray.includes(element)) {
+			return true;
+		}
+	} while (!(element.parent() instanceof SearchResult) && (element = <RenderableMatch>element.parent()));
+
+	return false;
+}

src/vs/workbench/contrib/search/test/common/searchResult.test.ts

@@ -7,7 +7,7 @@ import * as sinon from 'sinon';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { Match, FileMatch, SearchResult, SearchModel } from 'vs/workbench/contrib/search/common/searchModel';
 import { URI } from 'vs/base/common/uri';
-import { IFileMatch, TextSearchMatch, OneLineRange, ITextSearchMatch } from 'vs/workbench/services/search/common/search';
+import { IFileMatch, TextSearchMatch, OneLineRange, ITextSearchMatch, QueryType } from 'vs/workbench/services/search/common/search';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils';
 import { Range } from 'vs/editor/common/core/range';
@@ -37,7 +37,7 @@ suite('SearchResult', () => {
 		instantiationService.stub(IModelService, stubModelService(instantiationService));
 		instantiationService.stub(IUriIdentityService, new UriIdentityService(new FileService(new NullLogService())));
 		instantiationService.stubPromise(IReplaceService, {});
-		instantiationService.stubPromise(IReplaceService, 'replace', null);
+		instantiationService.stub(IReplaceService, 'replace', () => Promise.resolve(null));
 		instantiationService.stub(ILabelService, new MockLabelService());
 	});
 
@@ -342,6 +342,56 @@ suite('SearchResult', () => {
 		return voidPromise.then(() => assert.ok(testObject.isEmpty()));
 	});
 
+	test('batchRemove should trigger the onChange event correctly', function () {
+		const target = sinon.spy();
+		const testObject = getPopulatedSearchResult();
+
+		const folderMatch = testObject.folderMatches()[0];
+		const fileMatch = testObject.folderMatches()[1].matches()[0];
+		const match = testObject.folderMatches()[1].matches()[1].matches()[0];
+
+		const arrayToRemove = [folderMatch, fileMatch, match];
+		const expectedArrayResult = folderMatch.matches().concat([fileMatch, match.parent()]);
+
+		testObject.onChange(target);
+		testObject.batchRemove(arrayToRemove);
+
+		assert.ok(target.calledOnce);
+		assert.deepStrictEqual([{ elements: expectedArrayResult, removed: true, added: false }], target.args[0]);
+	});
+
+	test('batchReplace should trigger the onChange event correctly', async function () {
+		const replaceSpy = sinon.spy();
+		instantiationService.stub(IReplaceService, 'replace', (arg: any) => {
+			if (Array.isArray(arg)) {
+				replaceSpy(arg[0]);
+			} else {
+				replaceSpy(arg);
+			}
+			return Promise.resolve();
+		});
+
+		const target = sinon.spy();
+		const testObject = getPopulatedSearchResult();
+
+		const folderMatch = testObject.folderMatches()[0];
+		const fileMatch = testObject.folderMatches()[1].matches()[0];
+		const match = testObject.folderMatches()[1].matches()[1].matches()[0];
+
+		const firstExpectedMatch = folderMatch.matches()[0];
+
+		const arrayToRemove = [folderMatch, fileMatch, match];
+
+		testObject.onChange(target);
+		await testObject.batchReplace(arrayToRemove);
+
+		assert.ok(target.calledOnce);
+		sinon.assert.calledThrice(replaceSpy);
+		sinon.assert.calledWith(replaceSpy.firstCall, firstExpectedMatch);
+		sinon.assert.calledWith(replaceSpy.secondCall, fileMatch);
+		sinon.assert.calledWith(replaceSpy.thirdCall, match);
+	});
+
 	function aFileMatch(path: string, searchResult?: SearchResult, ...lineMatches: ITextSearchMatch[]): FileMatch {
 		const rawMatch: IFileMatch = {
 			resource: URI.file('/' + path),
@@ -365,4 +415,28 @@ suite('SearchResult', () => {
 		instantiationService.stub(IThemeService, new TestThemeService());
 		return instantiationService.createInstance(ModelService);
 	}
+
+	function getPopulatedSearchResult() {
+		const testObject = aSearchResult();
+
+		testObject.query = {
+			type: QueryType.Text,
+			contentPattern: { pattern: 'foo' },
+			folderQueries: [{
+				folder: URI.parse('file://c:/voo')
+			},
+			{ folder: URI.parse('file://c:/with') },
+			]
+		};
+
+		testObject.add([
+			aRawMatch('file://c:/voo/foo.a',
+				new TextSearchMatch('preview 1', lineOneRange), new TextSearchMatch('preview 2', lineOneRange)),
+			aRawMatch('file://c:/with/path/bar.b',
+				new TextSearchMatch('preview 3', lineOneRange)),
+			aRawMatch('file://c:/with/path.c',
+				new TextSearchMatch('preview 4', lineOneRange), new TextSearchMatch('preview 5', lineOneRange)),
+		]);
+		return testObject;
+	}
 });