Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache activated env variables if it takes >=500ms to activate #8363

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

DonJayamanne
Copy link
Contributor

For #7952

@DonJayamanne DonJayamanne requested a review from a team as a code owner November 24, 2021 22:20
* We cache activated environment variables.
* When activating environments, all activation scripts update environment variables, nothing else (after all they don't start a process).
* The env variables can change based on the activation script, current env variables on the machine & python interpreter information.
* If any of these change, then the env variables are invalidated.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heres an update of the logic

Copy link
Contributor Author

@DonJayamanne DonJayamanne Nov 24, 2021

Choose a reason for hiding this comment

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

This basically brings down activation from 14s to 5-6s.

@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2021

Codecov Report

Merging #8363 (7e5f669) into main (59c7231) will decrease coverage by 0%.
The diff coverage is 59%.

@@          Coverage Diff          @@
##            main   #8363   +/-   ##
=====================================
- Coverage     71%     71%   -1%     
=====================================
  Files        375     375           
  Lines      23746   23788   +42     
  Branches    3653    3662    +9     
=====================================
+ Hits       17080   17103   +23     
- Misses      5186    5199   +13     
- Partials    1480    1486    +6     
Impacted Files Coverage Δ
src/client/common/process/interpreterActivation.ts 72% <59%> (-4%) ⬇️
...ence/editor-integration/cellHashProviderFactory.ts 97% <0%> (-3%) ⬇️
.../datascience/editor-integration/codeLensFactory.ts 87% <0%> (-2%) ⬇️
src/client/datascience/jupyter/kernelVariables.ts 56% <0%> (-2%) ⬇️
src/client/datascience/baseJupyterSession.ts 76% <0%> (+<1%) ⬆️
src/client/common/process/currentProcess.ts 100% <0%> (+50%) ⬆️

@DonJayamanne DonJayamanne merged commit a7ad7c4 into main Nov 24, 2021
@DonJayamanne DonJayamanne deleted the warpFaster branch November 24, 2021 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants