Skip to content

Commit bb6aaee

Browse files
committed
address comments
1 parent 6093056 commit bb6aaee

2 files changed

Lines changed: 50 additions & 30 deletions

File tree

src/managers/common/fastPath.ts

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -91,52 +91,65 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
9191
}
9292

9393
// Look up the persisted path — either from workspace cache or global cache
94-
const persistedPath = isGlobalScope
95-
? await opts.getGlobalPersistedPath!()
96-
: await opts.getPersistedPath(opts.getProjectFsPath(opts.scope as Uri));
94+
if (isGlobalScope) {
95+
// Safe: guarded by the early return above
96+
const getGlobalPersistedPath = opts.getGlobalPersistedPath as () => Promise<string | undefined>;
9797

98-
// Track cross-session cache performance for global scope
99-
const cacheStopWatch = isGlobalScope ? new StopWatch() : undefined;
98+
// Track end-to-end cross-session cache performance for global scope, including persisted-path lookup.
99+
const cacheStopWatch = new StopWatch();
100+
const persistedPath = await getGlobalPersistedPath();
100101

101-
if (persistedPath) {
102-
try {
103-
const resolved = await opts.resolve(persistedPath);
104-
if (resolved) {
105-
if (isGlobalScope) {
106-
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch!.elapsedTime, {
102+
if (persistedPath) {
103+
try {
104+
const resolved = await opts.resolve(persistedPath);
105+
if (resolved) {
106+
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
107107
managerLabel: opts.label,
108108
result: 'hit',
109109
});
110+
return { env: resolved };
110111
}
111-
return { env: resolved };
112-
}
113-
// Cached path found but resolve returned undefined (e.g., Python was uninstalled)
114-
if (isGlobalScope) {
115-
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch!.elapsedTime, {
112+
// Cached path found but resolve returned undefined (e.g., Python was uninstalled)
113+
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
116114
managerLabel: opts.label,
117115
result: 'stale',
118116
});
119-
}
120-
} catch (err) {
121-
if (isGlobalScope) {
122-
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch!.elapsedTime, {
117+
} catch (err) {
118+
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
123119
managerLabel: opts.label,
124120
result: 'stale',
125121
});
122+
traceWarn(
123+
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
124+
err,
125+
);
126126
}
127-
traceWarn(
128-
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
129-
err,
130-
);
131-
}
132-
} else {
133-
if (isGlobalScope) {
134-
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch!.elapsedTime, {
127+
} else {
128+
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
135129
managerLabel: opts.label,
136130
result: 'miss',
137131
});
132+
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
133+
}
134+
} else {
135+
const scope = opts.scope as Uri;
136+
const persistedPath = await opts.getPersistedPath(opts.getProjectFsPath(scope));
137+
138+
if (persistedPath) {
139+
try {
140+
const resolved = await opts.resolve(persistedPath);
141+
if (resolved) {
142+
return { env: resolved };
143+
}
144+
} catch (err) {
145+
traceWarn(
146+
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
147+
err,
148+
);
149+
}
150+
} else {
151+
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
138152
}
139-
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
140153
}
141154

142155
return undefined;

src/test/managers/fastPath.get.unit.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ interface ManagerCase {
4242
name: string;
4343
managerId: string;
4444
persistedPath: string;
45+
supportsGlobalScope?: boolean;
4546
createContext: (sandbox: sinon.SinonSandbox) => ManagerCaseContext;
4647
}
4748

@@ -136,6 +137,7 @@ function createManagerCases(): ManagerCase[] {
136137
name: 'SysPythonManager',
137138
managerId: 'ms-python.python:system',
138139
persistedPath: path.resolve('test', 'bin', 'python3'),
140+
supportsGlobalScope: true,
139141
createContext: (sandbox: sinon.SinonSandbox) => {
140142
const getPersistedStub = sandbox.stub(sysCache, 'getSystemEnvForWorkspace');
141143
const resolveStub = sandbox.stub(sysUtils, 'resolveSystemPythonEnvironmentPath');
@@ -218,7 +220,12 @@ function runSharedFastPathTests(managerCase: ManagerCase, getSandbox: () => sino
218220

219221
await manager.get(undefined);
220222

221-
assert.ok(getPersistedStub.notCalled);
223+
// For managers that support global scope, the workspace getPersistedPath may be
224+
// called by background init (loadEnvMap), so we only assert notCalled for managers
225+
// that skip the fast path entirely when scope is undefined.
226+
if (!managerCase.supportsGlobalScope) {
227+
assert.ok(getPersistedStub.notCalled);
228+
}
222229
});
223230

224231
test('skip fast path: already initialized', async () => {

0 commit comments

Comments
 (0)