Skip to content

Commit 05201e6

Browse files
TyriarTyriar
authored
Merge pull request microsoft#259340 from microsoft/tyriar/258512
Add performance logging and improve performance in terminal suggest
2 parents e3f315d + 9eedc68 commit 05201e6

File tree

6 files changed

+99
-56
lines changed

6 files changed

+99
-56
lines changed

‎extensions/terminal-suggest/src/env/pathExecutableCache.ts

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -105,55 +105,57 @@ export class PathExecutableCache implements vscode.Disposable {
105105
const result = new Set<ICompletionResource>();
106106
const fileResource = vscode.Uri.file(path);
107107
const files = await vscode.workspace.fs.readDirectory(fileResource);
108-
for (const [file, fileType] of files) {
109-
let kind: vscode.TerminalCompletionItemKind | undefined;
110-
let formattedPath: string | undefined;
111-
const resource = vscode.Uri.joinPath(fileResource, file);
112-
113-
// Skip unknown or directory file types early
114-
if (fileType === vscode.FileType.Unknown || fileType === vscode.FileType.Directory) {
115-
continue;
116-
}
108+
await Promise.all(
109+
files.map(([file, fileType]) => (async () => {
110+
let kind: vscode.TerminalCompletionItemKind | undefined;
111+
let formattedPath: string | undefined;
112+
const resource = vscode.Uri.joinPath(fileResource, file);
113+
114+
// Skip unknown or directory file types early
115+
if (fileType === vscode.FileType.Unknown || fileType === vscode.FileType.Directory) {
116+
return;
117+
}
117118

118-
try {
119-
const lstat = await fs.lstat(resource.fsPath);
120-
if (lstat.isSymbolicLink()) {
121-
try {
122-
const symlinkRealPath = await fs.realpath(resource.fsPath);
123-
const isExec = await isExecutable(symlinkRealPath, this._cachedWindowsExeExtensions);
124-
if (!isExec) {
125-
continue;
119+
try {
120+
const lstat = await fs.lstat(resource.fsPath);
121+
if (lstat.isSymbolicLink()) {
122+
try {
123+
const symlinkRealPath = await fs.realpath(resource.fsPath);
124+
const isExec = await isExecutable(symlinkRealPath, this._cachedWindowsExeExtensions);
125+
if (!isExec) {
126+
return;
127+
}
128+
kind = vscode.TerminalCompletionItemKind.Method;
129+
formattedPath = `${resource.fsPath} -> ${symlinkRealPath}`;
130+
} catch {
131+
return;
126132
}
127-
kind = vscode.TerminalCompletionItemKind.Method;
128-
formattedPath = `${resource.fsPath} -> ${symlinkRealPath}`;
129-
} catch {
130-
continue;
131133
}
134+
} catch {
135+
// Ignore errors for unreadable files
136+
return;
132137
}
133-
} catch {
134-
// Ignore errors for unreadable files
135-
continue;
136-
}
137138

138-
formattedPath = formattedPath ?? getFriendlyResourcePath(resource, pathSeparator);
139+
formattedPath = formattedPath ?? getFriendlyResourcePath(resource, pathSeparator);
139140

140-
// Check if already added or not executable
141-
if (labels.has(file)) {
142-
continue;
143-
}
141+
// Check if already added or not executable
142+
if (labels.has(file)) {
143+
return;
144+
}
144145

145-
const isExec = kind === vscode.TerminalCompletionItemKind.Method || await isExecutable(formattedPath, this._cachedWindowsExeExtensions);
146-
if (!isExec) {
147-
continue;
148-
}
146+
const isExec = kind === vscode.TerminalCompletionItemKind.Method || await isExecutable(formattedPath, this._cachedWindowsExeExtensions);
147+
if (!isExec) {
148+
return;
149+
}
149150

150-
result.add({
151-
label: file,
152-
documentation: formattedPath,
153-
kind: kind ?? vscode.TerminalCompletionItemKind.Method
154-
});
155-
labels.add(file);
156-
}
151+
result.add({
152+
label: file,
153+
documentation: formattedPath,
154+
kind: kind ?? vscode.TerminalCompletionItemKind.Method
155+
});
156+
labels.add(file);
157+
})())
158+
);
157159
return result;
158160
} catch (e) {
159161
// Ignore errors for directories that can't be read

‎extensions/terminal-suggest/src/fig/figInterface.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,26 @@ export async function getFigSuggestions(
4646
items: [],
4747
};
4848
const currentCommand = currentCommandAndArgString.split(' ')[0];
49+
50+
// Assemble a map to allow O(1) access to the available command from a spec
51+
// label. The label does not include an extension on Windows.
52+
const specLabelToAvailableCommandMap = new Map<string, ICompletionResource>();
53+
for (const command of availableCommands) {
54+
let label = typeof command.label === 'string' ? command.label : command.label.label;
55+
if (osIsWindows()) {
56+
label = removeAnyFileExtension(label);
57+
}
58+
specLabelToAvailableCommandMap.set(label, command);
59+
}
60+
4961
for (const spec of specs) {
5062
const specLabels = getFigSuggestionLabel(spec);
5163

5264
if (!specLabels) {
5365
continue;
5466
}
5567
for (const specLabel of specLabels) {
56-
const availableCommand = (osIsWindows()
57-
? availableCommands.find(command => (typeof command.label === 'string' ? command.label : command.label.label).match(new RegExp(`${specLabel}(\\.[^ ]+)?$`)))
58-
: availableCommands.find(command => (typeof command.label === 'string' ? command.label : command.label.label) === (specLabel)));
68+
const availableCommand = specLabelToAvailableCommandMap.get(specLabel);
5969
if (!availableCommand || (token && token.isCancellationRequested)) {
6070
continue;
6171
}

‎extensions/terminal-suggest/src/terminalSuggestMain.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ export async function activate(context: vscode.ExtensionContext) {
295295
}
296296
}
297297

298+
298299
if (terminal.shellIntegration?.cwd && (result.filesRequested || result.foldersRequested)) {
299300
return new vscode.TerminalCompletionList(result.items, {
300301
filesRequested: result.filesRequested,

‎src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalCompletionService.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@ import { IConfigurationService } from '../../../../../platform/configuration/com
1212
import { IFileService } from '../../../../../platform/files/common/files.js';
1313
import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js';
1414
import { TerminalCapability, type ITerminalCapabilityStore } from '../../../../../platform/terminal/common/capabilities/capabilities.js';
15-
import { GeneralShellType, TerminalShellType, WindowsShellType } from '../../../../../platform/terminal/common/terminal.js';
15+
import { GeneralShellType, ITerminalLogService, TerminalShellType, WindowsShellType } from '../../../../../platform/terminal/common/terminal.js';
1616
import { TerminalSuggestSettingId } from '../common/terminalSuggestConfiguration.js';
1717
import { TerminalCompletionItemKind, type ITerminalCompletion } from './terminalCompletionItem.js';
1818
import { env as processEnv } from '../../../../../base/common/process.js';
1919
import type { IProcessEnvironment } from '../../../../../base/common/platform.js';
2020
import { timeout } from '../../../../../base/common/async.js';
2121
import { gitBashToWindowsPath } from './terminalGitBashHelpers.js';
2222
import { isEqual } from '../../../../../base/common/resources.js';
23-
import { ILogService } from '../../../../../platform/log/common/log.js';
2423

2524
export const ITerminalCompletionService = createDecorator<ITerminalCompletionService>('terminalCompletionService');
2625

@@ -104,7 +103,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
104103
constructor(
105104
@IConfigurationService private readonly _configurationService: IConfigurationService,
106105
@IFileService private readonly _fileService: IFileService,
107-
@ILogService private readonly _logService: ILogService
106+
@ITerminalLogService private readonly _logService: ITerminalLogService
108107
) {
109108
super();
110109
}
@@ -132,6 +131,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
132131
}
133132

134133
async provideCompletions(promptValue: string, cursorPosition: number, allowFallbackCompletions: boolean, shellType: TerminalShellType | undefined, capabilities: ITerminalCapabilityStore, token: CancellationToken, triggerCharacter?: boolean, skipExtensionCompletions?: boolean, explicitlyInvoked?: boolean): Promise<ITerminalCompletion[] | undefined> {
134+
this._logService.trace('TerminalCompletionService#provideCompletions');
135135
if (!this._providers || !this._providers.values || cursorPosition < 0) {
136136
return undefined;
137137
}
@@ -178,6 +178,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
178178
}
179179

180180
private async _collectCompletions(providers: ITerminalCompletionProvider[], shellType: TerminalShellType | undefined, promptValue: string, cursorPosition: number, allowFallbackCompletions: boolean, capabilities: ITerminalCapabilityStore, token: CancellationToken, explicitlyInvoked?: boolean): Promise<ITerminalCompletion[] | undefined> {
181+
this._logService.trace('TerminalCompletionService#_collectCompletions');
181182
const completionPromises = providers.map(async provider => {
182183
if (provider.shellTypes && shellType && !provider.shellTypes.includes(shellType)) {
183184
return undefined;
@@ -187,7 +188,10 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
187188
let completions;
188189
try {
189190
completions = await Promise.race([
190-
provider.provideCompletions(promptValue, cursorPosition, allowFallbackCompletions, token),
191+
provider.provideCompletions(promptValue, cursorPosition, allowFallbackCompletions, token).then(result => {
192+
this._logService.trace(`TerminalCompletionService#_collectCompletions provider ${provider.id} finished`);
193+
return result;
194+
}),
191195
(async () => { await timeout(timeoutMs); timedOut = true; return undefined; })()
192196
]);
193197
} catch (e) {
@@ -202,6 +206,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
202206
return undefined;
203207
}
204208
const completionItems = Array.isArray(completions) ? completions : completions.items ?? [];
209+
this._logService.trace(`TerminalCompletionService#_collectCompletions amend ${completionItems.length} completion items`);
205210
if (shellType === GeneralShellType.PowerShell) {
206211
for (const completion of completionItems) {
207212
completion.isFileOverride ??= completion.kind === TerminalCompletionItemKind.Method && completion.replacementIndex === 0;
@@ -218,24 +223,29 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
218223
}
219224
if (completions.resourceRequestConfig) {
220225
const resourceCompletions = await this.resolveResources(completions.resourceRequestConfig, promptValue, cursorPosition, `core:path:ext:${provider.id}`, capabilities, shellType);
226+
this._logService.trace(`TerminalCompletionService#_collectCompletions dedupe`);
221227
if (resourceCompletions) {
228+
const labels = new Set(completionItems.map(c => c.label));
222229
for (const item of resourceCompletions) {
223-
const labels = new Set(completionItems.map(c => c.label));
224230
// Ensure no duplicates such as .
225231
if (!labels.has(item.label)) {
226232
completionItems.push(item);
227233
}
228234
}
229235
}
236+
this._logService.trace(`TerminalCompletionService#_collectCompletions dedupe done`);
230237
}
231238
return completionItems;
232239
});
233240

234241
const results = await Promise.all(completionPromises);
242+
this._logService.trace('TerminalCompletionService#_collectCompletions done');
235243
return results.filter(result => !!result).flat();
236244
}
237245

238246
async resolveResources(resourceRequestConfig: TerminalResourceRequestConfig, promptValue: string, cursorPosition: number, provider: string, capabilities: ITerminalCapabilityStore, shellType?: TerminalShellType): Promise<ITerminalCompletion[] | undefined> {
247+
this._logService.trace(`TerminalCompletionService#resolveResources`);
248+
239249
const useWindowsStylePath = resourceRequestConfig.pathSeparator === '\\';
240250
if (useWindowsStylePath) {
241251
// for tests, make sure the right path separator is used
@@ -360,6 +370,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
360370
// - (absolute) `/src/|` -> `/src/`
361371
// - (tilde) `~/|` -> `~/`
362372
// - (tilde) `~/src/|` -> `~/src/`
373+
this._logService.trace(`TerminalCompletionService#resolveResources cwd`);
363374
if (foldersRequested) {
364375
let label: string;
365376
switch (type) {
@@ -394,7 +405,8 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
394405
// - (relative) `cd ./src/` -> `cd ./src/folder1/`, ...
395406
// - (absolute) `cd c:/src/` -> `cd c:/src/folder1/`, ...
396407
// - (tilde) `cd ~/src/` -> `cd ~/src/folder1/`, ...
397-
for (const child of stat.children) {
408+
this._logService.trace(`TerminalCompletionService#resolveResources direct children`);
409+
await Promise.all(stat.children.map(child => (async () => {
398410
let kind: TerminalCompletionItemKind | undefined;
399411
let detail: string | undefined = undefined;
400412
if (foldersRequested && child.isDirectory) {
@@ -411,7 +423,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
411423
}
412424
}
413425
if (kind === undefined) {
414-
continue;
426+
return;
415427
}
416428

417429
let label = lastWordFolder;
@@ -431,7 +443,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
431443
if (child.isFile && fileExtensions) {
432444
const extension = child.name.split('.').length > 1 ? child.name.split('.').at(-1) : undefined;
433445
if (extension && !fileExtensions.includes(extension)) {
434-
continue;
446+
return;
435447
}
436448
}
437449

@@ -455,11 +467,12 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
455467
replacementIndex: cursorPosition - lastWord.length,
456468
replacementLength: lastWord.length
457469
});
458-
}
470+
})()));
459471

460472
// Support $CDPATH specially for the `cd` command only
461473
//
462474
// - (relative) `|` -> `/foo/vscode` (CDPATH has /foo which contains vscode folder)
475+
this._logService.trace(`TerminalCompletionService#resolveResources CDPATH`);
463476
if (type === 'relative' && foldersRequested) {
464477
if (promptValue.startsWith('cd ')) {
465478
const config = this._configurationService.getValue(TerminalSuggestSettingId.CdPath);
@@ -500,6 +513,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
500513
//
501514
// - (relative) `|` -> `../`
502515
// - (relative) `./src/|` -> `./src/../`
516+
this._logService.trace(`TerminalCompletionService#resolveResources parent dir`);
503517
if (type === 'relative' && foldersRequested) {
504518
let label = `..${resourceRequestConfig.pathSeparator}`;
505519
if (lastWordFolder.length > 0) {
@@ -520,6 +534,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
520534
// input.
521535
//
522536
// - (relative) `|` -> `~`
537+
this._logService.trace(`TerminalCompletionService#resolveResources tilde`);
523538
if (type === 'relative' && !lastWordFolder.match(/[\\\/]/)) {
524539
let homeResource: URI | string | undefined;
525540
const home = this._getHomeDir(useWindowsStylePath, capabilities);
@@ -541,6 +556,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
541556
});
542557
}
543558

559+
this._logService.trace(`TerminalCompletionService#resolveResources done`);
544560
return resourceCompletions;
545561
}
546562

‎src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalSuggestAddon.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
249249
}
250250

251251
private async _handleCompletionProviders(terminal: Terminal | undefined, token: CancellationToken, explicitlyInvoked?: boolean): Promise<void> {
252+
this._logService.trace('SuggestAddon#_handleCompletionProviders');
253+
252254
// Nothing to handle if the terminal is not attached
253255
if (!terminal?.element || !this._enableWidget || !this._promptInputModel) {
254256
return;
@@ -273,6 +275,7 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
273275
}
274276

275277
if (!doNotRequestExtensionCompletions) {
278+
this._logService.trace('SuggestAddon#_handleCompletionProviders onTerminalCompletionsRequested');
276279
await this._extensionService.activateByEvent('onTerminalCompletionsRequested');
277280
}
278281
this._currentPromptInputState = {
@@ -295,7 +298,9 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
295298

296299
const quickSuggestionsConfig = this._configurationService.getValue<ITerminalSuggestConfiguration>(terminalSuggestConfigSection).quickSuggestions;
297300
const allowFallbackCompletions = explicitlyInvoked || quickSuggestionsConfig.unknown === 'on';
301+
this._logService.trace('SuggestAddon#_handleCompletionProviders provideCompletions');
298302
const providedCompletions = await this._terminalCompletionService.provideCompletions(this._currentPromptInputState.prefix, this._currentPromptInputState.cursorIndex, allowFallbackCompletions, this.shellType, this._capabilities, token, false, doNotRequestExtensionCompletions, explicitlyInvoked);
303+
this._logService.trace('SuggestAddon#_handleCompletionProviders provideCompletions done');
299304

300305
if (token.isCancellationRequested) {
301306
return;
@@ -351,10 +356,13 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
351356
items.push(this._inlineCompletionItem);
352357
}
353358

359+
this._logService.trace('TerminalCompletionService#_collectCompletions create model');
354360
const model = new TerminalCompletionModel(
355361
items,
356362
lineContext
357363
);
364+
this._logService.trace('TerminalCompletionService#_collectCompletions create model done');
365+
358366
if (token.isCancellationRequested) {
359367
this._completionRequestTimestamp = undefined;
360368
return;
@@ -388,6 +396,7 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
388396
}
389397

390398
async requestCompletions(explicitlyInvoked?: boolean): Promise<void> {
399+
this._logService.trace('SuggestAddon#requestCompletions');
391400
if (!this._promptInputModel) {
392401
this._shouldSyncWhenReady = true;
393402
return;
@@ -707,11 +716,15 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
707716
}
708717

709718
private _showCompletions(model: TerminalCompletionModel, explicitlyInvoked?: boolean): void {
719+
this._logService.trace('SuggestAddon#_showCompletions');
710720
if (!this._terminal?.element) {
711721
return;
712722
}
713723
const suggestWidget = this._ensureSuggestWidget(this._terminal);
724+
725+
this._logService.trace('SuggestAddon#_showCompletions setCompletionModel');
714726
suggestWidget.setCompletionModel(model);
727+
715728
this._register(suggestWidget.onDidFocus(() => this._terminal?.focus()));
716729
if (!this._promptInputModel || !explicitlyInvoked && model.items.length === 0) {
717730
return;
@@ -731,6 +744,7 @@ export class SuggestAddon extends Disposable implements ITerminalAddon, ISuggest
731744
}
732745
this._completionRequestTimestamp = undefined;
733746
}
747+
this._logService.trace('SuggestAddon#_showCompletions suggestWidget.showSuggestions');
734748
suggestWidget.showSuggestions(0, false, !explicitlyInvoked, cursorPosition);
735749
}
736750

‎src/vs/workbench/contrib/terminalContrib/suggest/test/browser/terminalCompletionService.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import { ShellEnvDetectionCapability } from '../../../../../../platform/terminal
1818
import { TerminalCapability } from '../../../../../../platform/terminal/common/capabilities/capabilities.js';
1919
import { ITerminalCompletion, TerminalCompletionItemKind } from '../../browser/terminalCompletionItem.js';
2020
import { count } from '../../../../../../base/common/strings.js';
21-
import { WindowsShellType } from '../../../../../../platform/terminal/common/terminal.js';
21+
import { ITerminalLogService, WindowsShellType } from '../../../../../../platform/terminal/common/terminal.js';
2222
import { gitBashToWindowsPath, windowsToGitBashPath } from '../../browser/terminalGitBashHelpers.js';
23-
import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js';
23+
import { NullLogService } from '../../../../../../platform/log/common/log.js';
2424
import { TerminalSuggestSettingId } from '../../common/terminalSuggestConfiguration.js';
2525

2626
const pathSeparator = isWindows ? '\\' : '/';
@@ -107,7 +107,7 @@ suite('TerminalCompletionService', () => {
107107
setup(() => {
108108
instantiationService = store.add(new TestInstantiationService());
109109
configurationService = new TestConfigurationService();
110-
instantiationService.stub(ILogService, new NullLogService());
110+
instantiationService.stub(ITerminalLogService, new NullLogService());
111111
instantiationService.stub(IConfigurationService, configurationService);
112112
instantiationService.stub(IFileService, {
113113
async stat(resource) {

0 commit comments

Comments
 (0)