Introduce Caching to the Command Executor #2110
Open
Conversation
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
Every time we run a command we spawn a new child process, which is expensive. Commands may differ a lot based on the CWD, shell, and other factors. However, with many extensions relying on us, C# and C# DevKit for example, often make us call 'dotnet_executable_path --info' with the same settings. This is a lot of overhead which can be cached. We can't cache it for too long because if someone edits the system, the result of dotnet --info may change. But most of the calls to info etc happen at the launch of the window which is also where perf is more vital. We add a setting to disable the cache or make it extended longer. This pattern allows caching in the future. We rely on node-cache which is a reputable library and MIT license. For everything that we want to be cached, we send the TTL for that command to the executor via options.dotnetInstallToolCacheTtlMs. That is separated into a different commit. The easiest way to test this is to open our extension log and see that things get cached and that changes depending on the ttl setting.
nagilson
commented
Feb 3, 2025
nagilson
commented
Feb 3, 2025
nagilson
commented
Feb 4, 2025
it could return all runtimes since the env wasnt set thus ignoring the setting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Every time we run a command we spawn a new child process, which is expensive. Commands may differ a lot based on the CWD, shell, and other factors. However, with many extensions relying on us, C# and C# DevKit for example, often make us call 'dotnet_executable_path --info' with the same settings. This is a lot of overhead which can be cached. That's one of many benefits that was made in us owning and sharing this code in the ecosystem in the first place.
We can't cache it for too long because if someone edits the system, the result of dotnet --info may change. Edits to the PATH won't get picked up because all process spawns from vscode will be children of the main process, so they won't get those updates. The host output can change though as long as the host was on the PATH.
Most of the calls to info etc happen at the launch of vscode which is also where perf is more vital.
Changes
We add a setting to disable the cache or make it extended longer. This pattern allows caching in the future.
We rely on node-cache which is a reputable library and MIT license. Adds this and updates some dependencies.
For everything that we want to be cached, we send the TTL for that command to the executor via options.dotnetInstallToolCacheTtlMs. That is separated into a different commit. Certain types of commands get an assigned number to make it easier to adjust them on the fly. The numbers are moderately arbitrary and subject to change (e.g. I assume most calls to dotnet --info happen in the first 5 seconds, and it will take about 5 seconds for that system state to change where a new version is installed.)
Testing & Experimentation
The easiest way to test this is to open our extension log and see that things get cached and that changes depending on the ttl setting.