Skip to content

Commit 099445b

Browse files
committed
debt - avoid statSync when computing workspace identifier
1 parent 4745ae6 commit 099445b

File tree

7 files changed

+50
-49
lines changed

7 files changed

+50
-49
lines changed

src/vs/code/electron-main/main.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { DiagnosticsService } from 'vs/platform/diagnostics/node/diagnosticsServ
3434
import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
3535
import { EnvironmentMainService, IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService';
3636
import { addArg, parseMainProcessArgv } from 'vs/platform/environment/node/argvHelper';
37-
import { createWaitMarkerFile } from 'vs/platform/environment/node/wait';
37+
import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait';
3838
import { IFileService } from 'vs/platform/files/common/files';
3939
import { FileService } from 'vs/platform/files/common/fileService';
4040
import { DiskFileSystemProvider } from 'vs/platform/files/node/diskFileSystemProvider';
@@ -469,8 +469,9 @@ class CodeMain {
469469
//
470470
// Note: we are not doing this if the wait marker has been already
471471
// added as argument. This can happen if Code was started from CLI.
472+
472473
if (args.wait && !args.waitMarkerFilePath) {
473-
const waitMarkerFilePath = createWaitMarkerFile(args.verbose);
474+
const waitMarkerFilePath = createWaitMarkerFileSync(args.verbose);
474475
if (waitMarkerFilePath) {
475476
addArg(process.argv, '--waitMarkerFilePath', waitMarkerFilePath);
476477
args.waitMarkerFilePath = waitMarkerFilePath;

src/vs/code/node/cli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
1919
import { buildHelpMessage, buildVersionMessage, OPTIONS } from 'vs/platform/environment/node/argv';
2020
import { addArg, parseCLIProcessArgv } from 'vs/platform/environment/node/argvHelper';
2121
import { getStdinFilePath, hasStdinWithoutTty, readFromStdin, stdinDataListener } from 'vs/platform/environment/node/stdin';
22-
import { createWaitMarkerFile } from 'vs/platform/environment/node/wait';
22+
import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait';
2323
import product from 'vs/platform/product/common/product';
2424
import { CancellationTokenSource } from 'vs/base/common/cancellation';
2525
import { randomPath } from 'vs/base/common/extpath';
@@ -220,7 +220,7 @@ export async function main(argv: string[]): Promise<any> {
220220
// is closed and then exit the waiting process.
221221
let waitMarkerFilePath: string | undefined;
222222
if (args.wait) {
223-
waitMarkerFilePath = createWaitMarkerFile(verbose);
223+
waitMarkerFilePath = createWaitMarkerFileSync(verbose);
224224
if (waitMarkerFilePath) {
225225
addArg(argv, '--waitMarkerFilePath', waitMarkerFilePath);
226226
}

src/vs/platform/diagnostics/electron-main/diagnosticsMainService.ts

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -42,33 +42,33 @@ export class DiagnosticsMainService implements IDiagnosticsMainService {
4242

4343
async getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<(IRemoteDiagnosticInfo | IRemoteDiagnosticError)[]> {
4444
const windows = this.windowsMainService.getWindows();
45-
const diagnostics: Array<IDiagnosticInfo | IRemoteDiagnosticError | undefined> = await Promise.all(windows.map(window => {
46-
return new Promise<IDiagnosticInfo | IRemoteDiagnosticError | undefined>((resolve) => {
47-
const remoteAuthority = window.remoteAuthority;
48-
if (remoteAuthority) {
49-
const replyChannel = `vscode:getDiagnosticInfoResponse${window.id}`;
50-
const args: IDiagnosticInfoOptions = {
51-
includeProcesses: options.includeProcesses,
52-
folders: options.includeWorkspaceMetadata ? this.getFolderURIs(window) : undefined
53-
};
54-
55-
window.sendWhenReady('vscode:getDiagnosticInfo', CancellationToken.None, { replyChannel, args });
56-
57-
validatedIpcMain.once(replyChannel, (_: IpcEvent, data: IRemoteDiagnosticInfo) => {
58-
// No data is returned if getting the connection fails.
59-
if (!data) {
60-
resolve({ hostName: remoteAuthority, errorMessage: `Unable to resolve connection to '${remoteAuthority}'.` });
61-
}
62-
63-
resolve(data);
64-
});
65-
66-
setTimeout(() => {
67-
resolve({ hostName: remoteAuthority, errorMessage: `Connection to '${remoteAuthority}' could not be established` });
68-
}, 5000);
69-
} else {
70-
resolve(undefined);
71-
}
45+
const diagnostics: Array<IDiagnosticInfo | IRemoteDiagnosticError | undefined> = await Promise.all(windows.map(async window => {
46+
const remoteAuthority = window.remoteAuthority;
47+
if (!remoteAuthority) {
48+
return undefined;
49+
}
50+
51+
const replyChannel = `vscode:getDiagnosticInfoResponse${window.id}`;
52+
const args: IDiagnosticInfoOptions = {
53+
includeProcesses: options.includeProcesses,
54+
folders: options.includeWorkspaceMetadata ? await this.getFolderURIs(window) : undefined
55+
};
56+
57+
return new Promise<IDiagnosticInfo | IRemoteDiagnosticError>(resolve => {
58+
window.sendWhenReady('vscode:getDiagnosticInfo', CancellationToken.None, { replyChannel, args });
59+
60+
validatedIpcMain.once(replyChannel, (_: IpcEvent, data: IRemoteDiagnosticInfo) => {
61+
// No data is returned if getting the connection fails.
62+
if (!data) {
63+
resolve({ hostName: remoteAuthority, errorMessage: `Unable to resolve connection to '${remoteAuthority}'.` });
64+
}
65+
66+
resolve(data);
67+
});
68+
69+
setTimeout(() => {
70+
resolve({ hostName: remoteAuthority, errorMessage: `Connection to '${remoteAuthority}' could not be established` });
71+
}, 5000);
7272
});
7373
}));
7474

@@ -82,7 +82,7 @@ export class DiagnosticsMainService implements IDiagnosticsMainService {
8282
for (const window of BrowserWindow.getAllWindows()) {
8383
const codeWindow = this.windowsMainService.getWindowById(window.id);
8484
if (codeWindow) {
85-
windows.push(this.codeWindowToInfo(codeWindow));
85+
windows.push(await this.codeWindowToInfo(codeWindow));
8686
} else {
8787
windows.push(this.browserWindowToInfo(window));
8888
}
@@ -97,8 +97,8 @@ export class DiagnosticsMainService implements IDiagnosticsMainService {
9797
};
9898
}
9999

100-
private codeWindowToInfo(window: ICodeWindow): IWindowDiagnostics {
101-
const folderURIs = this.getFolderURIs(window);
100+
private async codeWindowToInfo(window: ICodeWindow): Promise<IWindowDiagnostics> {
101+
const folderURIs = await this.getFolderURIs(window);
102102
const win = assertIsDefined(window.win);
103103

104104
return this.browserWindowToInfo(win, folderURIs, window.remoteAuthority);
@@ -113,14 +113,14 @@ export class DiagnosticsMainService implements IDiagnosticsMainService {
113113
};
114114
}
115115

116-
private getFolderURIs(window: ICodeWindow): URI[] {
116+
private async getFolderURIs(window: ICodeWindow): Promise<URI[]> {
117117
const folderURIs: URI[] = [];
118118

119119
const workspace = window.openedWorkspace;
120120
if (isSingleFolderWorkspaceIdentifier(workspace)) {
121121
folderURIs.push(workspace.uri);
122122
} else if (isWorkspaceIdentifier(workspace)) {
123-
const resolvedWorkspace = this.workspacesManagementMainService.resolveLocalWorkspaceSync(workspace.configPath); // workspace folders can only be shown for local (resolved) workspaces
123+
const resolvedWorkspace = await this.workspacesManagementMainService.resolveLocalWorkspace(workspace.configPath); // workspace folders can only be shown for local (resolved) workspaces
124124
if (resolvedWorkspace) {
125125
const rootFolders = resolvedWorkspace.folders;
126126
rootFolders.forEach(root => {

src/vs/platform/environment/node/wait.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { writeFileSync } from 'fs';
77
import { tmpdir } from 'os';
88
import { randomPath } from 'vs/base/common/extpath';
99

10-
export function createWaitMarkerFile(verbose?: boolean): string | undefined {
10+
export function createWaitMarkerFileSync(verbose?: boolean): string | undefined {
1111
const randomWaitMarkerPath = randomPath(tmpdir());
1212

1313
try {

src/vs/platform/workspaces/node/workspaces.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { createHash } from 'crypto';
7-
import { Stats, statSync } from 'fs';
7+
import { Stats } from 'fs';
88
import { Schemas } from 'vs/base/common/network';
99
import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
1010
import { originalFSPath } from 'vs/base/common/resources';
@@ -53,13 +53,15 @@ export function getSingleFolderWorkspaceIdentifier(folderUri: URI, folderStat?:
5353
return createHash('md5').update(folderUri.toString()).digest('hex');
5454
}
5555

56-
// Local: produce a hash from the path and include creation time as salt
56+
// Local: we use the ctime as extra salt to the
57+
// identifier so that folders getting recreated
58+
// result in a different identifier. However, if
59+
// the stat is not provided we return `undefined`
60+
// to ensure identifiers are stable for the given
61+
// URI.
62+
5763
if (!folderStat) {
58-
try {
59-
folderStat = statSync(folderUri.fsPath);
60-
} catch (error) {
61-
return undefined; // folder does not exist
62-
}
64+
return undefined;
6365
}
6466

6567
let ctime: number | undefined;
@@ -75,8 +77,6 @@ export function getSingleFolderWorkspaceIdentifier(folderUri: URI, folderStat?:
7577
}
7678
}
7779

78-
// we use the ctime as extra salt to the ID so that we catch the case of a folder getting
79-
// deleted and recreated. in that case we do not want to carry over previous state
8080
return createHash('md5').update(folderUri.fsPath).update(ctime ? String(ctime) : '').digest('hex');
8181
}
8282

src/vs/platform/workspaces/test/electron-main/workspaces.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ flakySuite('Workspaces', () => {
4141
fs.mkdirSync(path.join(testDir, 'f1'));
4242

4343
const localExistingUri = URI.file(path.join(testDir, 'f1'));
44-
const localExistingUriId = getSingleFolderWorkspaceIdentifier(localExistingUri);
44+
const localExistingUriId = getSingleFolderWorkspaceIdentifier(localExistingUri, fs.statSync(localExistingUri.fsPath));
4545
assert.ok(localExistingUriId?.id);
4646
});
4747

src/vs/server/node/server.cli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { cwd } from 'vs/base/common/process';
1212
import { dirname, extname, resolve, join } from 'vs/base/common/path';
1313
import { parseArgs, buildHelpMessage, buildVersionMessage, OPTIONS, OptionDescriptions, ErrorReporter } from 'vs/platform/environment/node/argv';
1414
import { NativeParsedArgs } from 'vs/platform/environment/common/argv';
15-
import { createWaitMarkerFile } from 'vs/platform/environment/node/wait';
15+
import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait';
1616
import { PipeCommand } from 'vs/workbench/api/node/extHostCLIServer';
1717
import { hasStdinWithoutTty, getStdinFilePath, readFromStdin } from 'vs/platform/environment/node/stdin';
1818

@@ -306,7 +306,7 @@ export async function main(desc: ProductDescription, args: string[]): Promise<vo
306306
console.log('At least one file must be provided to wait for.');
307307
return;
308308
}
309-
waitMarkerFilePath = createWaitMarkerFile(verbose);
309+
waitMarkerFilePath = createWaitMarkerFileSync(verbose);
310310
}
311311

312312
sendToPipe({

0 commit comments

Comments
 (0)