Skip to content

Commit 58f8f73

Browse files
nagilsonnagilson
authored
Improve Command Caching by Aliasing Commands (#2304)
* Wip to cache more * Switch approaches to alias a command root This simplifies the logic significantly. Some downsides to this: - its not bidirectional, so 'C:\dotnet' gets aliased to 'dotnet' but any cached stuff for 'C:\dotnet' is not saved and reused by 'dotnet'. But this is a problem with the previous approach. - Doing it this way allows us to future proof commands. e.g. list sdks, list runtimes, etc. having it bi-directional requires us to see what commands were already cached with the previous command key, which would mean we'd need to store all of the cached commands or search the entire cache map since the options etc are part of what gets cached, we can't just search via the command root, unless we separate the data structure into a 2 part lookup, but that will add further issues. consider linux edge cases here, this should be ok for now. * Fix ordering and add tests log seems to show its not working. Need to check this and added test. maybe the in place edit of the object doesnt work * Fix the logic and tests * Fix the sorting logic, thx copilot for writing this boring code
1 parent 303bb37 commit 58f8f73

File tree

5 files changed

+215
-40
lines changed

5 files changed

+215
-40
lines changed

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts‎

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { DotnetConditionsValidated, DotnetFindPathDidNotMeetCondition, DotnetUnableToCheckPATHArchitecture } from '../EventStream/EventStreamEvents';
66
import { IDotnetFindPathContext } from '../IDotnetFindPathContext';
77
import { CommandExecutor } from '../Utils/CommandExecutor';
8+
import { CommandExecutorCommand } from '../Utils/CommandExecutorCommand';
89
import { FileUtilities } from '../Utils/FileUtilities';
910
import { ICommandExecutor } from '../Utils/ICommandExecutor';
1011
import { IUtilityContext } from '../Utils/IUtilityContext';
@@ -29,41 +30,59 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
2930
public async dotnetMeetsRequirement(dotnetExecutablePath: string, requirement: IDotnetFindPathContext): Promise<boolean>
3031
{
3132
let hostArch = '';
33+
const oldLookup = process.env.DOTNET_MULTILEVEL_LOOKUP;
34+
// This is deprecated but still needed to scan .NET 6 and below
35+
process.env.DOTNET_MULTILEVEL_LOOKUP = '0'; // make it so --list-runtimes only finds the runtimes on that path: https://learn.microsoft.com/en-us/dotnet/core/compatibility/deployment/7.0/multilevel-lookup#reason-for-change
3236

33-
if (requirement.acquireContext.mode === 'sdk')
37+
try
3438
{
35-
const availableSDKs = await this.getSDKs(dotnetExecutablePath, requirement.acquireContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());
36-
hostArch = availableSDKs?.at(0)?.architecture ?? await this.getHostArchitecture(dotnetExecutablePath, requirement);
37-
if (availableSDKs.some((sdk) =>
39+
if (requirement.acquireContext.mode === 'sdk')
3840
{
39-
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) &&
40-
this.stringVersionMeetsRequirement(sdk.version, requirement.acquireContext.version, requirement) && this.allowPreview(sdk.version, requirement);
41-
}))
41+
const availableSDKs = await this.getSDKs(dotnetExecutablePath, requirement.acquireContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());
42+
hostArch = availableSDKs?.at(0)?.architecture ?? await this.getHostArchitecture(dotnetExecutablePath, requirement);
43+
if (availableSDKs.some((sdk) =>
44+
{
45+
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) &&
46+
this.stringVersionMeetsRequirement(sdk.version, requirement.acquireContext.version, requirement) && this.allowPreview(sdk.version, requirement);
47+
}))
48+
{
49+
this.workerContext.eventStream.post(new DotnetConditionsValidated(`${dotnetExecutablePath} satisfies the conditions.`));
50+
return true;
51+
}
52+
}
53+
else
4254
{
43-
this.workerContext.eventStream.post(new DotnetConditionsValidated(`${dotnetExecutablePath} satisfies the conditions.`));
44-
return true;
55+
// No need to consider SDKs when looking for runtimes as all the runtimes installed with the SDKs will be included in the runtimes list.
56+
const availableRuntimes = await this.getRuntimes(dotnetExecutablePath, requirement.acquireContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());
57+
hostArch = availableRuntimes?.at(0)?.architecture ?? await this.getHostArchitecture(dotnetExecutablePath, requirement);
58+
if (availableRuntimes.some((runtime) =>
59+
{
60+
return runtime.mode === requirement.acquireContext.mode && this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) &&
61+
this.stringVersionMeetsRequirement(runtime.version, requirement.acquireContext.version, requirement) && this.allowPreview(runtime.version, requirement);
62+
}))
63+
{
64+
this.workerContext.eventStream.post(new DotnetConditionsValidated(`${dotnetExecutablePath} satisfies the conditions.`));
65+
return true;
66+
}
4567
}
68+
69+
this.workerContext.eventStream.post(new DotnetFindPathDidNotMeetCondition(`${dotnetExecutablePath} did NOT satisfy the conditions: hostArch: ${hostArch}, requiredArch: ${requirement.acquireContext.architecture},
70+
required version: ${requirement.acquireContext.version}, required mode: ${requirement.acquireContext.mode}`));
71+
72+
return false;
4673
}
47-
else
74+
finally
4875
{
49-
// No need to consider SDKs when looking for runtimes as all the runtimes installed with the SDKs will be included in the runtimes list.
50-
const availableRuntimes = await this.getRuntimes(dotnetExecutablePath, requirement.acquireContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());
51-
hostArch = availableRuntimes?.at(0)?.architecture ?? await this.getHostArchitecture(dotnetExecutablePath, requirement);
52-
if (availableRuntimes.some((runtime) =>
76+
// Restore the environment variable to its original value
77+
if (oldLookup !== undefined)
5378
{
54-
return runtime.mode === requirement.acquireContext.mode && this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) &&
55-
this.stringVersionMeetsRequirement(runtime.version, requirement.acquireContext.version, requirement) && this.allowPreview(runtime.version, requirement);
56-
}))
79+
process.env.DOTNET_MULTILEVEL_LOOKUP = oldLookup;
80+
}
81+
else
5782
{
58-
this.workerContext.eventStream.post(new DotnetConditionsValidated(`${dotnetExecutablePath} satisfies the conditions.`));
59-
return true;
83+
delete process.env.DOTNET_MULTILEVEL_LOOKUP;
6084
}
6185
}
62-
63-
this.workerContext.eventStream.post(new DotnetFindPathDidNotMeetCondition(`${dotnetExecutablePath} did NOT satisfy the conditions: hostArch: ${hostArch}, requiredArch: ${requirement.acquireContext.architecture},
64-
required version: ${requirement.acquireContext.version}, required mode: ${requirement.acquireContext.mode}`));
65-
66-
return false;
6786
}
6887

6988
/**
@@ -303,6 +322,11 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement
303322
return true;
304323
}
305324

325+
private getRuntimesCommand(existingPath: string, requestedArchitecture: string): CommandExecutorCommand
326+
{
327+
return CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-runtimes', '--arch', requestedArchitecture]);
328+
}
329+
306330
public async getRuntimes(existingPath: string, requestedArchitecture: string | null): Promise<IDotnetListInfo[]>
307331
{
308332
if (!existingPath || existingPath === '""')
@@ -311,13 +335,12 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement
311335
}
312336

313337
requestedArchitecture ??= DotnetCoreAcquisitionWorker.defaultArchitecture()
314-
const findRuntimesCommand = CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-runtimes', '--arch', requestedArchitecture]);
315338

316339
const windowsDesktopString = 'Microsoft.WindowsDesktop.App';
317340
const aspnetCoreString = 'Microsoft.AspNetCore.App';
318341
const runtimeString = 'Microsoft.NETCore.App';
319342

320-
const runtimeInfo = await (this.executor!).execute(findRuntimesCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false).then(async (result) =>
343+
const runtimeInfo = await (this.executor!).execute(this.getRuntimesCommand(existingPath, requestedArchitecture), { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false).then(async (result) =>
321344
{
322345
if (result.status !== '0')
323346
{

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ import
3030
DotnetFindPathRootUnderEmulationButNoneSet,
3131
FileDoesNotExist
3232
} from '../EventStream/EventStreamEvents';
33+
import { LocalMemoryCacheSingleton } from '../LocalMemoryCacheSingleton';
3334
import { FileUtilities } from '../Utils/FileUtilities';
3435
import { IFileUtilities } from '../Utils/IFileUtilities';
3536
import { EnvironmentVariableIsDefined, getDotnetExecutable, getOSArch, getPathSeparator } from '../Utils/TypescriptUtilities';
3637
import { DOTNET_INFORMATION_CACHE_DURATION_MS, SYS_CMD_SEARCH_CACHE_DURATION_MS } from './CacheTimeConstants';
3738
import { DotnetConditionValidator } from './DotnetConditionValidator';
39+
import { DotnetCoreAcquisitionWorker } from './DotnetCoreAcquisitionWorker';
3840
import { InstallRecordWithPath } from './InstallRecordWithPath';
3941
import { InstallTrackerSingleton } from './InstallTrackerSingleton';
4042
import { RegistryReader } from './RegistryReader';
@@ -380,11 +382,13 @@ export class DotnetPathFinder implements IDotnetPathFinder
380382
public async getTruePath(tentativePaths: string[], requestedArchitecture: string | null): Promise<string[]>
381383
{
382384
const truePaths = [];
385+
requestedArchitecture ??= DotnetCoreAcquisitionWorker.defaultArchitecture()
383386

384387
for (const tentativePath of tentativePaths)
385388
{
386389
// This will even work if only the sdk is installed, list-runtimes on an sdk installed host would work
387-
const runtimeInfo = await new DotnetConditionValidator(this.workerContext, this.utilityContext, this.executor).getRuntimes(tentativePath, requestedArchitecture);
390+
const validator = new DotnetConditionValidator(this.workerContext, this.utilityContext, this.executor);
391+
const runtimeInfo = await validator.getRuntimes(tentativePath, requestedArchitecture);
388392
if ((runtimeInfo?.length ?? 0) > 0)
389393
{
390394
// q.t. from @dibarbet on the C# Extension:
@@ -396,7 +400,19 @@ export class DotnetPathFinder implements IDotnetPathFinder
396400
//
397401
// Since dotnet --list-runtimes will always use the real assembly path to output the runtime folder (no symlinks!)
398402
// we know the dotnet executable will be two folders up in the install root.
399-
truePaths.push(path.join(path.dirname(path.dirname(runtimeInfo[0].directory)), getDotnetExecutable()));
403+
const truePath = path.join(path.dirname(path.dirname(runtimeInfo[0].directory)), getDotnetExecutable());
404+
truePaths.push(truePath);
405+
406+
// Preload the cache with the calls we've already done.
407+
// Example: 'dotnet' --list-runtimes will be the same as 'C:\\Program Files\\dotnet\\dotnet.exe' --list-runtimes
408+
// If the dotnet executable full path was 'C:\\Program Files\\dotnet\\dotnet.exe'.
409+
410+
// We do NOT want to do this on Unix, because the dotnet executable is potentially polymorphic.
411+
// /usr/local/bin/dotnet becomes /snap/dotnet-sdk/current/dotnet in reality, may have different behavior in shells.
412+
if (os.platform() === 'win32')
413+
{
414+
LocalMemoryCacheSingleton.getInstance().aliasCommandAsAnotherCommandRoot(`"${truePath}"`, `"${tentativePath}"`, this.workerContext.eventStream);
415+
}
400416
}
401417
else
402418
{

‎vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,15 @@ export class DotnetFindPathLookupSetting extends DotnetCustomMessageEvent
11951195
public readonly eventName = 'DotnetFindPathLookupSetting';
11961196
}
11971197

1198+
export class CacheAliasCreated extends DotnetCustomMessageEvent
1199+
{
1200+
public readonly eventName = 'CacheAliasCreated';
1201+
public getProperties()
1202+
{
1203+
return { Message: this.eventMessage, ...getDisabledTelemetryOnChance(1) };
1204+
};
1205+
}
1206+
11981207
export class DotnetFindPathSettingFound extends DotnetCustomMessageEvent
11991208
{
12001209
public readonly eventName = 'DotnetFindPathSettingFound';

‎vscode-dotnet-runtime-library/src/LocalMemoryCacheSingleton.ts‎

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
import * as nodeCache from 'node-cache';
88
import { IAcquisitionWorkerContext } from "./Acquisition/IAcquisitionWorkerContext";
9-
import { CacheClearEvent, CacheGetEvent, CachePutEvent } from "./EventStream/EventStreamEvents";
10-
import { TelemetryUtilities } from './EventStream/TelemetryUtilities';
9+
import { IEventStream } from './EventStream/EventStream';
10+
import { CacheAliasCreated, CacheClearEvent, CacheGetEvent, CachePutEvent } from "./EventStream/EventStreamEvents";
1111
import { CommandExecutor } from "./Utils/CommandExecutor";
1212
import { CommandExecutorCommand } from "./Utils/CommandExecutorCommand";
1313
import { CommandExecutorResult } from "./Utils/CommandExecutorResult";
@@ -31,6 +31,8 @@ export class LocalMemoryCacheSingleton
3131

3232
protected cache: nodeCache = new nodeCache();
3333

34+
private commandRootAliases: Map<string, string> = new Map<string, string>();
35+
3436
protected constructor(public readonly timeToLiveMultiplier = 1)
3537
{
3638

@@ -67,6 +69,10 @@ export class LocalMemoryCacheSingleton
6769

6870
public getCommand(key: CacheableCommand, context: IAcquisitionWorkerContext): CommandExecutorResult | undefined
6971
{
72+
if (this.commandRootAliases.has(key.command.commandRoot))
73+
{
74+
key.command.commandRoot = this.commandRootAliases.get(key.command.commandRoot) ?? key.command.commandRoot;
75+
}
7076
return this.get(this.cacheableCommandToKey(key), context);
7177
}
7278

@@ -93,9 +99,28 @@ export class LocalMemoryCacheSingleton
9399
{
94100
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
95101
const ttl = key.options?.dotnetInstallToolCacheTtlMs ?? 5000;
102+
if (this.commandRootAliases.has(key.command.commandRoot))
103+
{
104+
key.command.commandRoot = this.commandRootAliases.get(key.command.commandRoot) ?? key.command.commandRoot;
105+
}
106+
96107
return this.put(this.cacheableCommandToKey(key), obj, { ttlMs: ttl } as LocalMemoryCacheMetadata, context);
97108
}
98109

110+
/**
111+
*
112+
* @param commandRootAlias The root of the command that will result in the same output. Example: if we know "dotnet" is "C:\\Program Files\\dotnet\\dotnet.exe"
113+
* @param aliasedCommandRoot The identical command that had likely already been cached.
114+
* @remarks This does not work in the opposite direction. If you alias "dotnet" to "dotnet2", it will not alias "dotnet2" to "dotnet".
115+
* Trying that will result in neither command being cache aliased to one another.
116+
* Basically, this will replace all command checks in the cache that have the commandRoot of `commandRootAlias` with the `aliasedCommandRoot`.
117+
*/
118+
public aliasCommandAsAnotherCommandRoot(commandRootAlias: string, aliasedCommandRoot: string, eventStream: IEventStream): void
119+
{
120+
eventStream.post(new CacheAliasCreated(`Aliasing command root ${commandRootAlias} to ${aliasedCommandRoot}`));
121+
this.commandRootAliases.set(commandRootAlias, aliasedCommandRoot);
122+
}
123+
99124
public invalidate(context?: IAcquisitionWorkerContext): void
100125
{
101126
context?.eventStream.post(new CacheClearEvent(`Wiping the cache at ${new Date().toISOString()}`));
@@ -104,19 +129,35 @@ export class LocalMemoryCacheSingleton
104129

105130
private cacheableCommandToKey(key: CacheableCommand): string
106131
{
107-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
108-
return `${CommandExecutor.prettifyCommandExecutorCommand(key.command)}${JSON.stringify(key.options, function replacer(k, v)
132+
// Get all keys sorted
133+
const sortedKeys = Object.keys(key.options).sort();
134+
135+
// Build ordered string manually
136+
let optionsString = '{';
137+
sortedKeys.forEach((k, index) =>
109138
{
110-
// Replace the dotnetInstallToolCacheTtlMs key with undefined so that it doesn't affect the cache key.
139+
const v = key.options[k];
140+
// Apply same replacer logic
141+
let value = v;
111142
if (k === 'dotnetInstallToolCacheTtlMs')
112143
{
113-
return undefined;
144+
value = undefined;
145+
} else if (k === 'env')
146+
{
147+
value = minimizeEnvironment(v);
114148
}
115-
else if (k === 'env')
149+
150+
if (value !== undefined)
116151
{
117-
return `${minimizeEnvironment(v)}`;
152+
optionsString += `"${k}":${JSON.stringify(value)}`;
153+
if (index < sortedKeys.length - 1)
154+
{
155+
optionsString += ',';
156+
}
118157
}
119-
return v;
120-
})}`;
158+
});
159+
optionsString += '}';
160+
161+
return `${CommandExecutor.prettifyCommandExecutorCommand(key.command)}${optionsString}`;
121162
}
122163
};

0 commit comments

Comments
 (0)