-
Notifications
You must be signed in to change notification settings - Fork 30.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #239398 from microsoft/tyriar/239396_speedup
Optimize getCommandsInPath, restore cache, clean up cache lifecycle, add test
- Loading branch information
Showing
6 changed files
with
176 additions
and
84 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 109 additions & 0 deletions
109
extensions/terminal-suggest/src/env/pathExecutableCache.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as fs from 'fs/promises'; | ||
| import * as vscode from 'vscode'; | ||
| import { isExecutable } from '../helpers/executable'; | ||
| import { osIsWindows } from '../helpers/os'; | ||
| import type { ICompletionResource } from '../types'; | ||
| import { getFriendlyResourcePath } from '../helpers/uri'; | ||
| import { SettingsIds } from '../constants'; | ||
|
|
||
| const isWindows = osIsWindows(); | ||
|
|
||
| export class PathExecutableCache implements vscode.Disposable { | ||
| private _disposables: vscode.Disposable[] = []; | ||
|
|
||
| private _cachedAvailableCommandsPath: string | undefined; | ||
| private _cachedWindowsExecutableExtensions: { [key: string]: boolean | undefined } | undefined; | ||
| private _cachedCommandsInPath: { completionResources: Set<ICompletionResource> | undefined; labels: Set<string> | undefined } | undefined; | ||
|
|
||
| constructor() { | ||
| if (isWindows) { | ||
| this._cachedWindowsExecutableExtensions = vscode.workspace.getConfiguration(SettingsIds.SuggestPrefix).get(SettingsIds.CachedWindowsExecutableExtensionsSuffixOnly); | ||
| this._disposables.push(vscode.workspace.onDidChangeConfiguration(e => { | ||
| if (e.affectsConfiguration(SettingsIds.CachedWindowsExecutableExtensions)) { | ||
| this._cachedWindowsExecutableExtensions = vscode.workspace.getConfiguration(SettingsIds.SuggestPrefix).get(SettingsIds.CachedWindowsExecutableExtensionsSuffixOnly); | ||
| this._cachedCommandsInPath = undefined; | ||
| } | ||
| })); | ||
| } | ||
| } | ||
|
|
||
| dispose() { | ||
| for (const d of this._disposables) { | ||
| d.dispose(); | ||
| } | ||
| } | ||
|
|
||
| async getCommandsInPath(env: { [key: string]: string | undefined } = process.env): Promise<{ completionResources: Set<ICompletionResource> | undefined; labels: Set<string> | undefined } | undefined> { | ||
| // Create cache key | ||
| let pathValue: string | undefined; | ||
| if (isWindows) { | ||
| const caseSensitivePathKey = Object.keys(env).find(key => key.toLowerCase() === 'path'); | ||
| if (caseSensitivePathKey) { | ||
| pathValue = env[caseSensitivePathKey]; | ||
| } | ||
| } else { | ||
| pathValue = env.PATH; | ||
| } | ||
| if (pathValue === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| // Check cache | ||
| if (this._cachedCommandsInPath && this._cachedAvailableCommandsPath === pathValue) { | ||
| return this._cachedCommandsInPath; | ||
| } | ||
|
|
||
| // Extract executables from PATH | ||
| const paths = pathValue.split(isWindows ? ';' : ':'); | ||
| const pathSeparator = isWindows ? '\\' : '/'; | ||
| const promises: Promise<Set<ICompletionResource> | undefined>[] = []; | ||
| const labels: Set<string> = new Set<string>(); | ||
| for (const path of paths) { | ||
| promises.push(this._getFilesInPath(path, pathSeparator, labels)); | ||
| } | ||
|
|
||
| // Merge all results | ||
| const executables = new Set<ICompletionResource>(); | ||
| const resultSets = await Promise.all(promises); | ||
| for (const resultSet of resultSets) { | ||
| if (resultSet) { | ||
| for (const executable of resultSet) { | ||
| executables.add(executable); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Return | ||
| this._cachedAvailableCommandsPath = pathValue; | ||
| this._cachedCommandsInPath = { completionResources: executables, labels }; | ||
| return this._cachedCommandsInPath; | ||
| } | ||
|
|
||
| private async _getFilesInPath(path: string, pathSeparator: string, labels: Set<string>): Promise<Set<ICompletionResource> | undefined> { | ||
| try { | ||
| const dirExists = await fs.stat(path).then(stat => stat.isDirectory()).catch(() => false); | ||
| if (!dirExists) { | ||
| return undefined; | ||
| } | ||
| const result = new Set<ICompletionResource>(); | ||
| const fileResource = vscode.Uri.file(path); | ||
| const files = await vscode.workspace.fs.readDirectory(fileResource); | ||
| for (const [file, fileType] of files) { | ||
| const formattedPath = getFriendlyResourcePath(vscode.Uri.joinPath(fileResource, file), pathSeparator); | ||
| if (!labels.has(file) && fileType !== vscode.FileType.Unknown && fileType !== vscode.FileType.Directory && await isExecutable(formattedPath, this._cachedWindowsExecutableExtensions)) { | ||
| result.add({ label: file, detail: formattedPath }); | ||
| labels.add(file); | ||
| } | ||
| } | ||
| return result; | ||
| } catch (e) { | ||
| // Ignore errors for directories that can't be read | ||
| return undefined; | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as vscode from 'vscode'; | ||
|
|
||
| export function getFriendlyResourcePath(uri: vscode.Uri, pathSeparator: string, kind?: vscode.TerminalCompletionItemKind): string { | ||
| let path = uri.fsPath; | ||
| // Ensure drive is capitalized on Windows | ||
| if (pathSeparator === '\\' && path.match(/^[a-zA-Z]:\\/)) { | ||
| path = `${path[0].toUpperCase()}:${path.slice(2)}`; | ||
| } | ||
| if (kind === vscode.TerminalCompletionItemKind.Folder) { | ||
| if (!path.endsWith(pathSeparator)) { | ||
| path += pathSeparator; | ||
| } | ||
| } | ||
| return path; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
extensions/terminal-suggest/src/test/env/pathExecutableCache.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import 'mocha'; | ||
| import { strictEqual } from 'node:assert'; | ||
| import { PathExecutableCache } from '../../env/pathExecutableCache'; | ||
|
|
||
| suite('PathExecutableCache', () => { | ||
| test('cache should return empty for empty PATH', async () => { | ||
| const cache = new PathExecutableCache(); | ||
| const result = await cache.getCommandsInPath({ PATH: '' }); | ||
| strictEqual(Array.from(result!.completionResources!).length, 0); | ||
| strictEqual(Array.from(result!.labels!).length, 0); | ||
| }); | ||
|
|
||
| test('caching is working on successive calls', async () => { | ||
| const cache = new PathExecutableCache(); | ||
| const env = { PATH: process.env.PATH }; | ||
| const result = await cache.getCommandsInPath(env); | ||
| const result2 = await cache.getCommandsInPath(env); | ||
| strictEqual(result, result2); | ||
| }); | ||
| }); |