Skip to content

Make resolving related to windows registry faster#16697

Merged
karrtikr merged 3 commits into
microsoft:mainfrom
karrtikr:registy
Jul 16, 2021
Merged

Make resolving related to windows registry faster#16697
karrtikr merged 3 commits into
microsoft:mainfrom
karrtikr:registy

Conversation

@karrtikr
Copy link
Copy Markdown

@karrtikr karrtikr commented Jul 15, 2021

@karrtikr karrtikr added the no-changelog No news entry required label Jul 15, 2021
resolvedEnv.source = uniq(resolvedEnv.source.concat(source ?? []));
if (getOSType() === OSType.Windows && resolvedEnv.source?.includes(PythonEnvSource.WindowsRegistry)) {
// We can update env further using information we can get from the Windows registry.
await updateEnvUsingRegistry(resolvedEnv);
Copy link
Copy Markdown
Author

@karrtikr karrtikr Jul 15, 2021

Choose a reason for hiding this comment

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

Weirdly, discovery actually got slower than before with #16642. It was because we were blocking resolving of all environments on getting registry interpreters, which effectively means all the locators are blocked on windows registry locator.

With this change we're only blocking resolving on registry for interpreters which were discovered via Windows registry.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you have an estimate (% or ms) on how much faster it is now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, it got approx. 30% faster with the promise caching and this.

const interpreters = await getRegistryInterpreters();
// Environment source has already been identified as windows registry, so we expect windows registry
// cache to already be populated. Call sync function which relies on cache.
let interpreters = getRegistryInterpretersSync();
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Having sync methods make updateEnvUsingRegistry finish faster as we do not yield control.

/**
* Environment was found via conda binary or conda environments file
*/
Conda = 'conda',
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed entries which were not being used anywhere.

@karrtikr karrtikr merged commit cdcf76c into microsoft:main Jul 16, 2021
@karrtikr karrtikr deleted the registy branch July 16, 2021 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants