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

Use worker threads for fetching Windows Registry interpreters #22479

Merged
merged 6 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ jobs:
with:
run: npm run testSingleWorkspace
working-directory: ${{ env.special-working-directory }}
if: matrix.test-suite == 'venv' && matrix.os == 'ubuntu-latest'
if: matrix.test-suite == 'venv'

- name: Run single-workspace tests
env:
Expand Down
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@
"pythonSurveyNotification",
"pythonPromptNewToolsExt",
"pythonTerminalEnvVarActivation",
"pythonDiscoveryUsingWorkers",
"pythonTestAdapter",
"pythonREPLSmartSend",
"pythonRecommendTensorboardExt"
Expand All @@ -547,6 +548,7 @@
"%python.experiments.pythonSurveyNotification.description%",
"%python.experiments.pythonPromptNewToolsExt.description%",
"%python.experiments.pythonTerminalEnvVarActivation.description%",
"%python.experiments.pythonDiscoveryUsingWorkers.description%",
"%python.experiments.pythonTestAdapter.description%",
"%python.experiments.pythonREPLSmartSend.description%",
"%python.experiments.pythonRecommendTensorboardExt.description%"
Expand All @@ -565,6 +567,7 @@
"pythonSurveyNotification",
"pythonPromptNewToolsExt",
"pythonTerminalEnvVarActivation",
"pythonDiscoveryUsingWorkers",
"pythonTestAdapter",
"pythonREPLSmartSend"
],
Expand All @@ -573,6 +576,7 @@
"%python.experiments.pythonSurveyNotification.description%",
"%python.experiments.pythonPromptNewToolsExt.description%",
"%python.experiments.pythonTerminalEnvVarActivation.description%",
"%python.experiments.pythonDiscoveryUsingWorkers.description%",
"%python.experiments.pythonTestAdapter.description%",
"%python.experiments.pythonREPLSmartSend.description%"
]
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"python.experiments.pythonSurveyNotification.description": "Denotes the Python Survey Notification experiment.",
"python.experiments.pythonPromptNewToolsExt.description": "Denotes the Python Prompt New Tools Extension experiment.",
"python.experiments.pythonTerminalEnvVarActivation.description": "Enables use of environment variables to activate terminals instead of sending activation commands.",
"python.experiments.pythonDiscoveryUsingWorkers.description": "Enables use of worker threads to do heavy computation when discovering interpreters.",
"python.experiments.pythonTestAdapter.description": "Denotes the Python Test Adapter experiment.",
"python.experiments.pythonREPLSmartSend.description": "Denotes the Python REPL Smart Send experiment.",
"python.experiments.pythonRecommendTensorboardExt.description": "Denotes the Tensorboard Extension recommendation experiment.",
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/experiments/groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ export enum TerminalEnvVarActivation {
experiment = 'pythonTerminalEnvVarActivation',
}

export enum DiscoveryUsingWorkers {
experiment = 'pythonDiscoveryUsingWorkers',
}

// Experiment to enable the new testing rewrite.
export enum EnableTestAdapterRewrite {
experiment = 'pythonTestAdapter',
Expand Down
2 changes: 1 addition & 1 deletion src/client/pythonEnvironments/base/locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export interface ILocator<I = PythonEnvInfo, E = PythonEnvsChangedEvent> extends
* @param query - if provided, the locator will limit results to match
* @returns - the fast async iterator of Python envs, which may have incomplete info
*/
iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator<I>;
iterEnvs(query?: QueryForEvent<E>, useWorkerThreads?: boolean): IPythonEnvsIterator<I>;
}

export type ICompositeLocator<I = PythonEnvInfo, E = PythonEnvsChangedEvent> = Omit<ILocator<I, E>, 'providerId'>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ async function updateEnvUsingRegistry(env: PythonEnvInfo): Promise<void> {
let interpreters = getRegistryInterpretersSync();
if (!interpreters) {
traceError('Expected registry interpreter cache to be initialized already');
interpreters = await getRegistryInterpreters();
interpreters = await getRegistryInterpreters(true);
}
const data = interpreters.find((i) => arePathsSame(i.interpreterPath, env.executable.filename));
if (data) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ export class WindowsRegistryLocator extends Locator<BasicEnvInfo> {
public readonly providerId: string = 'windows-registry';

// eslint-disable-next-line class-methods-use-this
public iterEnvs(): IPythonEnvsIterator<BasicEnvInfo> {
public iterEnvs(_?: unknown, useWorkerThreads = true): IPythonEnvsIterator<BasicEnvInfo> {
const iterator = async function* () {
traceVerbose('Searching for windows registry interpreters');
const interpreters = await getRegistryInterpreters();
const interpreters = await getRegistryInterpreters(useWorkerThreads);
for (const interpreter of interpreters) {
try {
// Filter out Microsoft Store app directories. We have a store app locator that handles this.
Expand Down
11 changes: 10 additions & 1 deletion src/client/pythonEnvironments/base/locators/wrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@

// eslint-disable-next-line max-classes-per-file
import { Uri } from 'vscode';
import { DiscoveryUsingWorkers } from '../../../common/experiments/groups';
import { IDisposable } from '../../../common/types';
import { iterEmpty } from '../../../common/utils/async';
import { getURIFilter } from '../../../common/utils/misc';
import { Disposables } from '../../../common/utils/resourceLifecycle';
import { traceLog } from '../../../logging';
import { inExperiment } from '../../common/externalDependencies';
import { PythonEnvInfo } from '../info';
import { BasicEnvInfo, ILocator, IPythonEnvsIterator, PythonLocatorQuery } from '../locator';
import { combineIterators, Locators } from '../locators';
Expand All @@ -17,6 +20,8 @@ import { LazyResourceBasedLocator } from './common/resourceBasedLocator';
*/

export class ExtensionLocators<I = PythonEnvInfo> extends Locators<I> {
private readonly useWorkerThreads: boolean;

constructor(
// These are expected to be low-level locators (e.g. system).
private readonly nonWorkspace: ILocator<I>[],
Expand All @@ -25,6 +30,10 @@ export class ExtensionLocators<I = PythonEnvInfo> extends Locators<I> {
private readonly workspace: ILocator<I>,
) {
super([...nonWorkspace, workspace]);
this.useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment);
if (this.useWorkerThreads) {
traceLog('Using worker threads for discovery...');
}
}

public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<I> {
Expand All @@ -33,7 +42,7 @@ export class ExtensionLocators<I = PythonEnvInfo> extends Locators<I> {
const nonWorkspace = query?.providerId
? this.nonWorkspace.filter((locator) => query.providerId === locator.providerId)
: this.nonWorkspace;
iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query)));
iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query, this.useWorkerThreads)));
}
return combineIterators(iterators);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export class Conda {
}

async function* getCandidatesFromRegistry() {
const interps = await getRegistryInterpreters();
const interps = await getRegistryInterpreters(true);
const candidates = interps
.filter((interp) => interp.interpreterPath && interp.distroOrgName === 'ContinuumAnalytics')
.map((interp) => path.join(path.win32.dirname(interp.interpreterPath), suffix));
Expand Down
30 changes: 29 additions & 1 deletion src/client/pythonEnvironments/common/externalDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

import * as fsapi from 'fs-extra';
import * as path from 'path';
import { Worker } from 'worker_threads';
import * as vscode from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { ExecutionResult, IProcessServiceFactory, ShellOptions, SpawnOptions } from '../../common/process/types';
import { IDisposable, IConfigurationService } from '../../common/types';
import { IDisposable, IConfigurationService, IExperimentService } from '../../common/types';
import { chain, iterable } from '../../common/utils/async';
import { getOSType, OSType } from '../../common/utils/platform';
import { IServiceContainer } from '../../ioc/types';
Expand All @@ -29,6 +30,11 @@ export async function exec(file: string, args: string[], options: SpawnOptions =
return service.exec(file, args, options);
}

export function inExperiment(experimentName: string): boolean {
const service = internalServiceContainer.get<IExperimentService>(IExperimentService);
return service.inExperimentSync(experimentName);
}

// Workspace

export function isVirtualWorkspace(): boolean {
Expand Down Expand Up @@ -195,3 +201,25 @@ export function onDidChangePythonSetting(name: string, callback: () => void, roo
}
});
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
export async function executeWorkerFile(workerFileName: string, workerData: any): Promise<any> {
return new Promise((resolve, reject) => {
const worker = new Worker(workerFileName, { workerData });
worker.on('message', (res: { err: Error; res: unknown }) => {
if (res.err) {
reject(res.err);
}
resolve(res.res);
});
worker.on('error', (ex: Error) => {
traceError(`Error in worker ${workerFileName}`, ex);
reject(ex);
});
worker.on('exit', (code) => {
if (code !== 0) {
reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`));
}
});
});
}
24 changes: 24 additions & 0 deletions src/client/pythonEnvironments/common/registryKeysWorker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Registry } from 'winreg';
import { parentPort, workerData } from 'worker_threads';
import { IRegistryKey } from './windowsRegistry';

const WinReg = require('winreg');

const regKey = new WinReg(workerData);

function copyRegistryKeys(keys: IRegistryKey[]): IRegistryKey[] {
// Use the map function to create a new array with copies of the specified properties.
return keys.map((key) => ({
hive: key.hive,
arch: key.arch,
key: key.key,
}));
}

regKey.keys((err: Error, res: Registry[]) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
const messageRes = copyRegistryKeys(res);
parentPort.postMessage({ err, res: messageRes });
});
27 changes: 27 additions & 0 deletions src/client/pythonEnvironments/common/registryValuesWorker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { RegistryItem } from 'winreg';
import { parentPort, workerData } from 'worker_threads';
import { IRegistryValue } from './windowsRegistry';

const WinReg = require('winreg');

const regKey = new WinReg(workerData);

function copyRegistryValues(values: IRegistryValue[]): IRegistryValue[] {
// Use the map function to create a new array with copies of the specified properties.
return values.map((value) => ({
hive: value.hive,
arch: value.arch,
key: value.key,
name: value.name,
type: value.type,
value: value.value,
}));
}

regKey.values((err: Error, res: RegistryItem[]) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
const messageRes = copyRegistryValues(res);
parentPort.postMessage({ err, res: messageRes });
});
57 changes: 33 additions & 24 deletions src/client/pythonEnvironments/common/windowsRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { HKCU, HKLM, Options, REG_SZ, Registry, RegistryItem } from 'winreg';
import * as path from 'path';
import { createDeferred } from '../../common/utils/async';
import { executeWorkerFile } from './externalDependencies';

export { HKCU, HKLM, REG_SZ, Options };

Expand All @@ -22,30 +25,36 @@ export interface IRegistryValue {
value: string;
}

export async function readRegistryValues(options: Options): Promise<IRegistryValue[]> {
// eslint-disable-next-line global-require
const WinReg = require('winreg');
const regKey = new WinReg(options);
const deferred = createDeferred<RegistryItem[]>();
regKey.values((err: Error, res: RegistryItem[]) => {
if (err) {
deferred.reject(err);
}
deferred.resolve(res);
});
return deferred.promise;
export async function readRegistryValues(options: Options, useWorkerThreads: boolean): Promise<IRegistryValue[]> {
if (!useWorkerThreads) {
// eslint-disable-next-line global-require
const WinReg = require('winreg');
const regKey = new WinReg(options);
const deferred = createDeferred<RegistryItem[]>();
regKey.values((err: Error, res: RegistryItem[]) => {
if (err) {
deferred.reject(err);
}
deferred.resolve(res);
});
return deferred.promise;
}
return executeWorkerFile(path.join(__dirname, 'registryValuesWorker.js'), options);
}

export async function readRegistryKeys(options: Options): Promise<IRegistryKey[]> {
// eslint-disable-next-line global-require
const WinReg = require('winreg');
const regKey = new WinReg(options);
const deferred = createDeferred<Registry[]>();
regKey.keys((err: Error, res: Registry[]) => {
if (err) {
deferred.reject(err);
}
deferred.resolve(res);
});
return deferred.promise;
export async function readRegistryKeys(options: Options, useWorkerThreads: boolean): Promise<IRegistryKey[]> {
if (!useWorkerThreads) {
// eslint-disable-next-line global-require
const WinReg = require('winreg');
const regKey = new WinReg(options);
const deferred = createDeferred<Registry[]>();
regKey.keys((err: Error, res: Registry[]) => {
if (err) {
deferred.reject(err);
}
deferred.resolve(res);
});
return deferred.promise;
}
return executeWorkerFile(path.join(__dirname, 'registryKeysWorker.js'), options);
}
Loading
Loading