Skip to content

Performance improvements to load times of extension #931

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

Merged
merged 1 commit into from
Mar 2, 2018

Conversation

DonJayamanne
Copy link

@DonJayamanne DonJayamanne commented Mar 2, 2018

@@ -134,21 +131,14 @@ export class InterpreterService implements Disposable, IInterpreterService {
return false;
}
if (activeWorkspace.configTarget === ConfigurationTarget.Workspace) {
return !await this.isPythonPathDefined(pythonPathInConfig!.workspaceValue);
return pythonPathInConfig!.workspaceValue === undefined || pythonPathInConfig!.workspaceValue === 'python';
Copy link
Author

Choose a reason for hiding this comment

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

@MikhailArkhipov
I reverted the change you made. I've added a catch in display/index.ts which would handle cases where interpreter is invalid (does not exist).
This fs check alone takes over 1.5 seconds on my Mac.

Choose a reason for hiding this comment

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

Single file existence check? Wow.


@injectable()
export class PipEnvService implements IInterpreterLocatorService {
export class PipEnvService extends CacheableLocatorService {
Copy link
Author

Choose a reason for hiding this comment

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

Modified to inherit CacheableLocatorService.
Something I should have picked up during code review, my fault.

Choose a reason for hiding this comment

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

Yeah I thought about cacheable too

@codecov
Copy link

codecov bot commented Mar 2, 2018

Codecov Report

Merging #931 into master will decrease coverage by 0.08%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #931      +/-   ##
==========================================
- Coverage   64.07%   63.99%   -0.09%     
==========================================
  Files         260      260              
  Lines       12044    12020      -24     
  Branches     2134     2132       -2     
==========================================
- Hits         7717     7692      -25     
- Misses       4318     4319       +1     
  Partials        9        9
Impacted Files Coverage Δ
src/client/interpreter/display/index.ts 98.11% <0%> (-1.89%) ⬇️
src/client/interpreter/interpreterService.ts 79.56% <50%> (-1.24%) ⬇️
...ent/interpreter/locators/services/pipEnvService.ts 68.08% <80%> (-3.13%) ⬇️
src/client/common/platform/fileSystem.ts 78.26% <0%> (-4.35%) ⬇️
...reter/locators/services/cacheableLocatorService.ts 91.83% <0%> (-4.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a04ce9...6d00216. Read the comment docs.

@DonJayamanne DonJayamanne merged commit c2e0430 into microsoft:master Mar 2, 2018
@DonJayamanne DonJayamanne deleted the perfImprovements branch March 2, 2018 21:33
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
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.

Improve extension load time
2 participants