Skip to content

Commit 5a2baf9

Browse files
andreamahandreamah
authored
Search Tree Performance - FolderMatches and FileMatches (microsoft#162257)
* Search Tree Performance - FolderMatches and FileMatches Fixes microsoft#162246
1 parent 719461b commit 5a2baf9

File tree

2 files changed

+50
-34
lines changed

2 files changed

+50
-34
lines changed

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -559,9 +559,10 @@ export class FolderMatch extends Disposable {
559559
if (fileMatch) {
560560
fileMatch.bindModel(model);
561561
} else {
562-
this.folderMatches().forEach(e => {
563-
e.bindModel(model);
564-
});
562+
const folderMatches = this.folderMatchesIterator();
563+
for (const elem of folderMatches) {
564+
elem.bindModel(model);
565+
}
565566
}
566567
}
567568

@@ -603,15 +604,15 @@ export class FolderMatch extends Disposable {
603604
}
604605

605606
matches(): (FileMatch | FolderMatchWithResource)[] {
606-
return [...this.fileMatches(), ...this.folderMatches()];
607+
return [...this.fileMatchesIterator(), ...this.folderMatchesIterator()];
607608
}
608609

609-
fileMatches(): FileMatch[] {
610-
return [...this._fileMatches.values()];
610+
fileMatchesIterator(): IterableIterator<FileMatch> {
611+
return this._fileMatches.values();
611612
}
612613

613-
folderMatches(): FolderMatchWithResource[] {
614-
return [...this._folderMatches.values()];
614+
folderMatchesIterator(): IterableIterator<FolderMatchWithResource> {
615+
return this._folderMatches.values();
615616
}
616617

617618
isEmpty(): boolean {
@@ -623,8 +624,10 @@ export class FolderMatch extends Disposable {
623624
if (directChildFileMatch) {
624625
return directChildFileMatch;
625626
}
626-
for (let i = 0; i < this.folderMatches().length; i++) {
627-
const match = this.folderMatches()[i].hasFileUriDownstream(uri);
627+
628+
const folderMatches = this.folderMatchesIterator();
629+
for (const elem of folderMatches) {
630+
const match = elem.hasFileUriDownstream(uri);
628631
if (match) {
629632
return match;
630633
}
@@ -633,8 +636,13 @@ export class FolderMatch extends Disposable {
633636
}
634637

635638
downstreamFileMatches(): FileMatch[] {
636-
const recursiveChildren = this.folderMatches().map(e => e.downstreamFileMatches()).flat();
637-
return [...this.fileMatches(), ...recursiveChildren];
639+
let recursiveChildren: FileMatch[] = [];
640+
const iterator = this.folderMatchesIterator();
641+
for (const elem of iterator) {
642+
recursiveChildren = recursiveChildren.concat(elem.downstreamFileMatches());
643+
}
644+
645+
return [...this.fileMatchesIterator(), ...recursiveChildren];
638646
}
639647

640648
private fileCount(): number {
@@ -712,7 +720,7 @@ export class FolderMatch extends Disposable {
712720
return false;
713721
}
714722

715-
private getFolderMatch(resource: URI): FolderMatchWithResource | undefined {
723+
public getFolderMatch(resource: URI): FolderMatchWithResource | undefined {
716724
const folderMatch = this._folderMatchesMap.findSubstr(resource);
717725
return folderMatch;
718726
}
@@ -770,7 +778,7 @@ export class FolderMatch extends Disposable {
770778

771779
const removed = [];
772780
for (const match of fileMatches as FileMatch[]) {
773-
if (this.fileMatches().includes(match)) {
781+
if (this._fileMatches.get(match.resource)) {
774782
this._fileMatches.delete(match.resource);
775783
if (dispose) {
776784
match.dispose();
@@ -877,7 +885,7 @@ export class FolderMatchWorkspaceRoot extends FolderMatchWithResource {
877885
const root = this.closestRoot ?? this;
878886
let parent: FolderMatch = this;
879887
for (let i = 0; i < fileMatchParentParts.length; i++) {
880-
let folderMatch: FolderMatchWithResource | undefined = parent.folderMatches().find(e => e.resource && (this.uriEquals(e.resource, fileMatchParentParts[i])));
888+
let folderMatch: FolderMatchWithResource | undefined = parent.getFolderMatch(fileMatchParentParts[i]);
881889
if (!folderMatch) {
882890
folderMatch = parent.createIntermediateFolderMatch(fileMatchParentParts[i], fileMatchParentParts[i].toString(), null, this._query, root);
883891
}

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

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import * as assert from 'assert';
66
import * as sinon from 'sinon';
77
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
8-
import { Match, FileMatch, SearchResult, SearchModel } from 'vs/workbench/contrib/search/common/searchModel';
8+
import { Match, FileMatch, SearchResult, SearchModel, FolderMatch } from 'vs/workbench/contrib/search/common/searchModel';
99
import { URI } from 'vs/base/common/uri';
1010
import { IFileMatch, TextSearchMatch, OneLineRange, ITextSearchMatch, QueryType } from 'vs/workbench/services/search/common/search';
1111
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
@@ -402,36 +402,36 @@ suite('SearchResult', () => {
402402
const root3 = testObject.folderMatches()[3];
403403

404404
const root0DownstreamFiles = root0.downstreamFileMatches();
405-
assert.deepStrictEqual(root0DownstreamFiles, root0.fileMatches().concat(root0.folderMatches()[0].fileMatches()));
406-
assert.deepStrictEqual(root0.folderMatches()[0].downstreamFileMatches(), root0.folderMatches()[0].fileMatches());
407-
assert.deepStrictEqual(root0.folderMatches()[0].fileMatches()[0].parent(), root0.folderMatches()[0]);
408-
assert.deepStrictEqual(root0.folderMatches()[0].parent(), root0);
409-
assert.deepStrictEqual(root0.folderMatches()[0].closestRoot, root0);
405+
assert.deepStrictEqual(root0DownstreamFiles, [...root0.fileMatchesIterator(), ...getFolderMatchAtIndex(root0, 0).fileMatchesIterator()]);
406+
assert.deepStrictEqual(getFolderMatchAtIndex(root0, 0).downstreamFileMatches(), Array.from(getFolderMatchAtIndex(root0, 0).fileMatchesIterator()));
407+
assert.deepStrictEqual(getFileMatchAtIndex(getFolderMatchAtIndex(root0, 0), 0).parent(), getFolderMatchAtIndex(root0, 0));
408+
assert.deepStrictEqual(getFolderMatchAtIndex(root0, 0).parent(), root0);
409+
assert.deepStrictEqual(getFolderMatchAtIndex(root0, 0).closestRoot, root0);
410410
root0DownstreamFiles.forEach((e) => {
411411
assert.deepStrictEqual(e.closestRoot, root0);
412412
});
413413

414414
const root1DownstreamFiles = root1.downstreamFileMatches();
415-
assert.deepStrictEqual(root1.downstreamFileMatches(), root1.fileMatches().concat(root1.folderMatches()[0].fileMatches())); // excludes the matches from nested root
416-
assert.deepStrictEqual(root1.folderMatches()[0].fileMatches()[0].parent(), root1.folderMatches()[0]);
415+
assert.deepStrictEqual(root1.downstreamFileMatches(), [...root1.fileMatchesIterator(), ...getFolderMatchAtIndex(root1, 0).fileMatchesIterator()]); // excludes the matches from nested root
416+
assert.deepStrictEqual(getFileMatchAtIndex(getFolderMatchAtIndex(root1, 0), 0).parent(), getFolderMatchAtIndex(root1, 0));
417417
root1DownstreamFiles.forEach((e) => {
418418
assert.deepStrictEqual(e.closestRoot, root1);
419419
});
420420

421421
const root2DownstreamFiles = root2.downstreamFileMatches();
422-
assert.deepStrictEqual(root2DownstreamFiles, root2.fileMatches());
423-
assert.deepStrictEqual(root2.fileMatches()[0].parent(), root2);
424-
assert.deepStrictEqual(root2.fileMatches()[0].closestRoot, root2);
422+
assert.deepStrictEqual(root2DownstreamFiles, Array.from(root2.fileMatchesIterator()));
423+
assert.deepStrictEqual(getFileMatchAtIndex(root2, 0).parent(), root2);
424+
assert.deepStrictEqual(getFileMatchAtIndex(root2, 0).closestRoot, root2);
425425

426426

427427
const root3DownstreamFiles = root3.downstreamFileMatches();
428-
const root3Level3Folder = root3.folderMatches()[0].folderMatches()[0];
429-
assert.deepStrictEqual(root3DownstreamFiles, [root3.fileMatches(), ...root3Level3Folder.folderMatches()[0].fileMatches(), ...root3Level3Folder.folderMatches()[1].fileMatches()].flat());
430-
assert.deepStrictEqual(root3Level3Folder.downstreamFileMatches(), root3.folderMatches()[0].downstreamFileMatches());
428+
const root3Level3Folder = getFolderMatchAtIndex(getFolderMatchAtIndex(root3, 0), 0);
429+
assert.deepStrictEqual(root3DownstreamFiles, [...root3.fileMatchesIterator(), ...getFolderMatchAtIndex(root3Level3Folder, 0).fileMatchesIterator(), ...getFolderMatchAtIndex(root3Level3Folder, 1).fileMatchesIterator()].flat());
430+
assert.deepStrictEqual(root3Level3Folder.downstreamFileMatches(), getFolderMatchAtIndex(root3, 0).downstreamFileMatches());
431431

432-
assert.deepStrictEqual(root3Level3Folder.folderMatches()[1].fileMatches()[0].parent(), root3Level3Folder.folderMatches()[1]);
433-
assert.deepStrictEqual(root3Level3Folder.folderMatches()[1].parent(), root3Level3Folder);
434-
assert.deepStrictEqual(root3Level3Folder.parent(), root3.folderMatches()[0]);
432+
assert.deepStrictEqual(getFileMatchAtIndex(getFolderMatchAtIndex(root3Level3Folder, 1), 0).parent(), getFolderMatchAtIndex(root3Level3Folder, 1));
433+
assert.deepStrictEqual(getFolderMatchAtIndex(root3Level3Folder, 1).parent(), root3Level3Folder);
434+
assert.deepStrictEqual(root3Level3Folder.parent(), getFolderMatchAtIndex(root3, 0));
435435

436436
root3DownstreamFiles.forEach((e) => {
437437
assert.deepStrictEqual(e.closestRoot, root3);
@@ -442,7 +442,7 @@ suite('SearchResult', () => {
442442
const target = sinon.spy();
443443
const testObject = getPopulatedSearchResultForTreeTesting();
444444

445-
const folderMatch = testObject.folderMatches()[3].folderMatches()[0].folderMatches()[0].folderMatches()[0];
445+
const folderMatch = getFolderMatchAtIndex(getFolderMatchAtIndex(getFolderMatchAtIndex(testObject.folderMatches()[3], 0), 0), 0);
446446

447447
const expectedArrayResult = folderMatch.downstreamFileMatches();
448448

@@ -456,7 +456,7 @@ suite('SearchResult', () => {
456456
const target = sinon.spy();
457457
const testObject = getPopulatedSearchResultForTreeTesting();
458458

459-
const folderMatch = testObject.folderMatches()[3].folderMatches()[0];
459+
const folderMatch = getFolderMatchAtIndex(testObject.folderMatches()[3], 0);
460460

461461
const expectedArrayResult = folderMatch.downstreamFileMatches();
462462

@@ -600,4 +600,12 @@ suite('SearchResult', () => {
600600
]);
601601
return testObject;
602602
}
603+
604+
function getFolderMatchAtIndex(parent: FolderMatch, index: number) {
605+
return Array.from(parent.folderMatchesIterator())[index];
606+
}
607+
608+
function getFileMatchAtIndex(parent: FolderMatch, index: number) {
609+
return Array.from(parent.fileMatchesIterator())[index];
610+
}
603611
});

0 commit comments

Comments
 (0)