From bae21689f1a9c01e33c52660e85905523c879f82 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 4 Apr 2016 14:10:37 +0200 Subject: [PATCH] Improper progress during search (fixes #4508) --- .../services/search/node/fileSearch.ts | 26 +++++++++---------- .../services/search/node/textSearch.ts | 26 ++++++++++++------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/vs/workbench/services/search/node/fileSearch.ts b/src/vs/workbench/services/search/node/fileSearch.ts index 33073851bd9..b268393b726 100644 --- a/src/vs/workbench/services/search/node/fileSearch.ts +++ b/src/vs/workbench/services/search/node/fileSearch.ts @@ -54,17 +54,17 @@ export class FileWalker { this.isCanceled = true; } - public walk(rootFolders: string[], extraFiles: string[], onResult: (result: ISerializedFileMatch) => void, done: (error: Error, isLimitHit: boolean) => void): void { + public walk(rootFolders: string[], extraFiles: string[], onResult: (result: ISerializedFileMatch, size: number) => void, done: (error: Error, isLimitHit: boolean) => void): void { // Support that the file pattern is a full path to a file that exists - this.checkFilePatternAbsoluteMatch((exists) => { + this.checkFilePatternAbsoluteMatch((exists, size) => { if (this.isCanceled) { return done(null, this.isLimitHit); } // Report result from file pattern if matching if (exists) { - onResult({ path: this.filePattern }); + onResult({ path: this.filePattern }, size); // Optimization: a match on an absolute path is a good result and we do not // continue walking the entire root paths array for other matches because @@ -92,14 +92,14 @@ export class FileWalker { } // Support relative paths to files from a root resource - return this.checkFilePatternRelativeMatch(absolutePath, (match) => { + return this.checkFilePatternRelativeMatch(absolutePath, (match, size) => { if (this.isCanceled || this.isLimitHit) { return perEntryCallback(null, null); } // Report result from file pattern if matching if (match) { - onResult({ path: match }); + onResult({ path: match }, size); } return this.doWalk(paths.normalize(absolutePath), '', files, onResult, perEntryCallback); @@ -111,17 +111,17 @@ export class FileWalker { }); } - private checkFilePatternAbsoluteMatch(clb: (exists: boolean) => void): void { + private checkFilePatternAbsoluteMatch(clb: (exists: boolean, size?: number) => void): void { if (!this.filePattern || !paths.isAbsolute(this.filePattern)) { return clb(false); } return fs.stat(this.filePattern, (error, stat) => { - return clb(!error && !stat.isDirectory()); // only existing files + return clb(!error && !stat.isDirectory(), stat && stat.size); // only existing files }); } - private checkFilePatternRelativeMatch(basePath: string, clb: (matchPath: string) => void): void { + private checkFilePatternRelativeMatch(basePath: string, clb: (matchPath: string, size?: number) => void): void { if (!this.filePattern || paths.isAbsolute(this.filePattern)) { return clb(null); } @@ -129,11 +129,11 @@ export class FileWalker { const absolutePath = paths.join(basePath, this.filePattern); return fs.stat(absolutePath, (error, stat) => { - return clb(!error && !stat.isDirectory() ? absolutePath : null); // only existing files + return clb(!error && !stat.isDirectory() ? absolutePath : null, stat && stat.size); // only existing files }); } - private doWalk(absolutePath: string, relativeParentPathWithSlashes: string, files: string[], onResult: (result: ISerializedFileMatch) => void, done: (error: Error, result: any) => void): void { + private doWalk(absolutePath: string, relativeParentPathWithSlashes: string, files: string[], onResult: (result: ISerializedFileMatch, size: number) => void, done: (error: Error, result: any) => void): void { // Execute tasks on each file in parallel to optimize throughput flow.parallel(files, (file: string, clb: (error: Error) => void): void => { @@ -208,7 +208,7 @@ export class FileWalker { return clb(null); // ignore file if max file size is hit } - this.matchFile(onResult, currentAbsolutePath, currentRelativePathWithSlashes); + this.matchFile(onResult, currentAbsolutePath, currentRelativePathWithSlashes, stat.size); } // Unwind @@ -224,7 +224,7 @@ export class FileWalker { }); } - private matchFile(onResult: (result: ISerializedFileMatch) => void, absolutePath: string, relativePathWithSlashes: string): void { + private matchFile(onResult: (result: ISerializedFileMatch, size: number) => void, absolutePath: string, relativePathWithSlashes: string, size?: number): void { if (this.isFilePatternMatch(relativePathWithSlashes) && (!this.includePattern || glob.match(this.includePattern, relativePathWithSlashes))) { this.resultCount++; @@ -235,7 +235,7 @@ export class FileWalker { if (!this.isLimitHit) { onResult({ path: absolutePath - }); + }, size); } } } diff --git a/src/vs/workbench/services/search/node/textSearch.ts b/src/vs/workbench/services/search/node/textSearch.ts index 276d7a46859..3452df30762 100644 --- a/src/vs/workbench/services/search/node/textSearch.ts +++ b/src/vs/workbench/services/search/node/textSearch.ts @@ -34,6 +34,7 @@ export class Engine implements ISearchEngine { private isDone: boolean; private total: number; private worked: number; + private progressed: number; private walkerError: Error; private walkerIsDone: boolean; private fileEncoding: string; @@ -48,6 +49,7 @@ export class Engine implements ISearchEngine { this.limitReached = false; this.maxResults = config.maxResults; this.worked = 0; + this.progressed = 0; this.total = 0; this.fileEncoding = encodingExists(config.fileEncoding) ? config.fileEncoding : UTF8; } @@ -60,14 +62,19 @@ export class Engine implements ISearchEngine { public search(onResult: (match: ISerializedFileMatch) => void, onProgress: (progress: IProgress) => void, done: (error: Error, isLimitHit: boolean) => void): void { let resultCounter = 0; + let progress = () => { + this.progressed++; + if (this.progressed % Engine.PROGRESS_FLUSH_CHUNK_SIZE === 0) { + onProgress({ total: this.total, worked: this.worked }); // buffer progress in chunks to reduce pressure + } + }; + let unwind = (processed: number) => { this.worked += processed; // Emit progress() unless we got canceled or hit the limit if (processed && !this.isDone && !this.isCanceled && !this.limitReached) { - if (this.worked % Engine.PROGRESS_FLUSH_CHUNK_SIZE === 0) { - onProgress({ total: this.total, worked: this.worked }); - } + progress(); } // Emit done() @@ -78,18 +85,17 @@ export class Engine implements ISearchEngine { }; // Walk over the file system - this.walker.walk(this.rootFolders, this.extraFiles, (result) => { - this.total++; + this.walker.walk(this.rootFolders, this.extraFiles, (result, size) => { + size = size ||  1; + this.total += size; // If the result is empty or we have reached the limit or we are canceled, ignore it if (this.limitReached || this.isCanceled) { - return unwind(1); + return unwind(size); } // Indicate progress to the outside - if (this.worked % Engine.PROGRESS_FLUSH_CHUNK_SIZE === 0) { - onProgress({ total: this.total, worked: this.worked }); - } + progress(); let fileMatch: FileMatch = null; @@ -98,7 +104,7 @@ export class Engine implements ISearchEngine { onResult(fileMatch.serialize()); } - return unwind(1); + return unwind(size); }; let perLineCallback = (line: string, lineNumber: number) => { -- GitLab