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

Remove global search path cache #1627

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Mar 12, 2021

Calls to this function only happen either on settings update (we do a bunch of work scanning files for other reasons that are no slower than calling Python), or in the import resolver (which has a cache for this call explicitly). Remove this cache so that editable installs modified while the LS is running are properly handled.

Last thing to finish microsoft/pylance-release#78.

@@ -29,8 +29,6 @@ interface PythonPathResult {
prefix: string;
}

const cachedSearchPaths = new Map<string, PythonPathResult>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we're caching enough in the import resolver that the removal of this cache won't matter. Out of curiosity, did you do any benchmarking to confirm this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't; I can try and clone your team's repo to test.

Note that the import resolver is already caching this call explicitly (was before per venv though that code got modified), so I wouldn't expect it to have changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't find your repo anymore, but on typeshed there is no difference in the numbers I get when using --stats.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not that concerned about it. I can test it after you land this change.

@jakebailey jakebailey merged commit 72382ca into microsoft:master Mar 12, 2021
@jakebailey jakebailey deleted the remove-interp-cache branch March 12, 2021 23:17
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.

2 participants