Skip to content

Optimize getCommandsInPath, restore cache, clean up cache lifecycle, add test#239398

Merged
Tyriar merged 4 commits intomainfrom
tyriar/239396_speedup
Feb 3, 2025
Merged

Optimize getCommandsInPath, restore cache, clean up cache lifecycle, add test#239398
Tyriar merged 4 commits intomainfrom
tyriar/239396_speedup

Conversation

@Tyriar
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar commented Feb 1, 2025

Changes:

  • Fix cache mechanism as pathValue never got stored
  • Cache labels instead of returning empty set
  • Walking each directory in separate promise and await via Promise.all
  • Don't create a promise on Windows when checking exe
  • Move path executable logic into own file and class to help manage lifecycle/testing
  • Add a test to prevent future breakages like we've had in the past

Fixes #239396


First request didn't change too much, but should in the worse case as it's no longer sequential:

image

Second before (~150ms):

image

Second after (~8ms):

image

Changes:

- Fix cache mechanism as pathValue never got stored
- Cache labels instead of returning empty set
- Walking each directory in separate promise and await via Promise.all
- Don't create a promise on Windows when checking exe

Fixes #239396
@Tyriar Tyriar added this to the February 2025 milestone Feb 1, 2025
@Tyriar Tyriar requested a review from meganrogge February 1, 2025 13:52
@Tyriar Tyriar self-assigned this Feb 1, 2025
@Tyriar Tyriar changed the title Optimize getCommandsInPath, restore cache Optimize getCommandsInPath, restore cache, clean up cache lifecycle Feb 1, 2025
@Tyriar Tyriar changed the title Optimize getCommandsInPath, restore cache, clean up cache lifecycle Optimize getCommandsInPath, restore cache, clean up cache lifecycle, add test Feb 1, 2025
@Tyriar Tyriar enabled auto-merge February 1, 2025 16:09
Copy link
Copy Markdown
Collaborator

@meganrogge meganrogge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏼

@Tyriar Tyriar merged commit 65bcaaf into main Feb 3, 2025
@Tyriar Tyriar deleted the tyriar/239396_speedup branch February 3, 2025 15:56
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Mar 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize getCommandsInPath

2 participants