Skip to content

Commit

Permalink
Use worker threads for fetching conda environments and interpreter re…
Browse files Browse the repository at this point in the history
…lated info (microsoft#22481)
  • Loading branch information
Kartik Raj authored Nov 19, 2023
1 parent 3c552f9 commit f6e1338
Show file tree
Hide file tree
Showing 43 changed files with 537 additions and 91 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ jobs:
shell: pwsh
if: matrix.test-suite == 'venv'
run: |
# 1. For `terminalActivation.testvirtualenvs.test.ts`
# 1. For `*.testvirtualenvs.test.ts`
if ('${{ matrix.os }}' -match 'windows-latest') {
$condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe
$condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ jobs:
shell: pwsh
if: matrix.test-suite == 'venv'
run: |
# 1. For `terminalActivation.testvirtualenvs.test.ts`
# 1. For `*.testvirtualenvs.test.ts`
if ('${{ matrix.os }}' -match 'windows-latest') {
$condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe
$condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda
Expand Down Expand Up @@ -451,7 +451,7 @@ jobs:
PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json'
shell: pwsh
run: |
# 1. For `terminalActivation.testvirtualenvs.test.ts`
# 1. For `*.testvirtualenvs.test.ts`
if ('${{ matrix.os }}' -match 'windows-latest') {
$condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe
$condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ log.log
**/node_modules
*.pyc
*.vsix
envVars.txt
**/.vscode/.ropeproject/**
**/testFiles/**/.cache/**
*.noseids
Expand Down
5 changes: 3 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"[python]": {
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports.isort": true
"source.fixAll.eslint": "explicit",
"source.organizeImports.isort": "explicit"
},
"editor.defaultFormatter": "ms-python.black-formatter",
},
Expand All @@ -51,7 +52,7 @@
"prettier.printWidth": 120,
"prettier.singleQuote": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
"source.fixAll.eslint": "explicit"
},
"python.languageServer": "Default",
"typescript.preferences.importModuleSpecifier": "relative",
Expand Down
8 changes: 8 additions & 0 deletions build/webpack/webpack.extension.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const config = {
target: 'node',
entry: {
extension: './src/client/extension.ts',
'shellExec.worker': './src/client/common/process/worker/shellExec.worker.ts',
'plainExec.worker': './src/client/common/process/worker/plainExec.worker.ts',
'registryKeys.worker': 'src/client/pythonEnvironments/common/registryKeys.worker.ts',
'registryValues.worker': 'src/client/pythonEnvironments/common/registryValues.worker.ts',
},
devtool: 'source-map',
node: {
Expand Down Expand Up @@ -51,6 +55,10 @@ const config = {
},
],
},
{
test: /\.worker\.js$/,
use: { loader: 'worker-loader' },
},
],
},
externals: [
Expand Down
31 changes: 31 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,7 @@
"typescript": "4.5.5",
"uuid": "^8.3.2",
"webpack": "^5.76.0",
"worker-loader": "^3.0.8",
"webpack-bundle-analyzer": "^4.5.0",
"webpack-cli": "^4.9.2",
"webpack-fix-default-import-plugin": "^1.0.3",
Expand Down
3 changes: 2 additions & 1 deletion src/client/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export namespace ThemeIcons {

export const DEFAULT_INTERPRETER_SETTING = 'python';

export const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined;
export const isCI =
process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined || process.env.GITHUB_ACTIONS === 'true';

export function isTestExecution(): boolean {
return process.env.VSC_PYTHON_CI_TEST === '1' || isUnitTestExecution();
Expand Down
9 changes: 8 additions & 1 deletion src/client/common/process/proc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IDisposable } from '../types';
import { EnvironmentVariables } from '../variables/types';
import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis';
import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types';
import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper';

export class ProcessService extends EventEmitter implements IProcessService {
private processesToKill = new Set<IDisposable>();
Expand Down Expand Up @@ -47,14 +48,20 @@ export class ProcessService extends EventEmitter implements IProcessService {
}

public exec(file: string, args: string[], options: SpawnOptions = {}): Promise<ExecutionResult<string>> {
this.emit('exec', file, args, options);
if (options.useWorker) {
return workerPlainExec(file, args, options);
}
const execOptions = { ...options, doNotLog: true };
const promise = plainExec(file, args, execOptions, this.env, this.processesToKill);
this.emit('exec', file, args, options);
return promise;
}

public shellExec(command: string, options: ShellOptions = {}): Promise<ExecutionResult<string>> {
this.emit('exec', command, undefined, options);
if (options.useWorker) {
return workerShellExec(command, options);
}
const disposables = new Set<IDisposable>();
const shellOptions = { ...options, doNotLog: true };
return shellExec(command, shellOptions, this.env, disposables).finally(() => {
Expand Down
1 change: 0 additions & 1 deletion src/client/common/process/rawProcessApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ export function shellExec(
disposables?: Set<IDisposable>,
): Promise<ExecutionResult<string>> {
const shellOptions = getDefaultOptions(options, defaultEnv);
traceVerbose(`Shell Exec: ${command} with options: ${JSON.stringify(shellOptions, null, 4)}`);
if (!options.doNotLog) {
const processLogger = new ProcessLogger(new WorkspaceService());
processLogger.logProcess(command, undefined, shellOptions);
Expand Down
3 changes: 2 additions & 1 deletion src/client/common/process/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
extraVariables?: NodeJS.ProcessEnv;
outputChannel?: OutputChannel;
stdinStr?: string;
useWorker?: boolean;
};

export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean; useWorker?: boolean };

export type ExecutionResult<T extends string | Buffer> = {
stdout: T;
Expand Down
42 changes: 42 additions & 0 deletions src/client/common/process/worker/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { Worker } from 'worker_threads';
import * as path from 'path';
import { traceVerbose, traceError } from '../../../logging/index';

/**
* Executes a worker file. Make sure to declare the worker file as a entry in the webpack config.
* @param workerFileName Filename of the worker file to execute, it has to end with ".worker.js" for webpack to bundle it.
* @param workerData Arguments to the worker file.
* @returns
*/
// 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> {
if (!workerFileName.endsWith('.worker.js')) {
throw new Error('Worker file must end with ".worker.js" for webpack to bundle webworkers');
}
return new Promise((resolve, reject) => {
const worker = new Worker(workerFileName, { workerData });
const id = worker.threadId;
traceVerbose(
`Worker id ${id} for file ${path.basename(workerFileName)} with data ${JSON.stringify(workerData)}`,
);
worker.on('message', (msg: { err: Error; res: unknown }) => {
if (msg.err) {
reject(msg.err);
}
resolve(msg.res);
});
worker.on('error', (ex: Error) => {
traceError(`Error in worker ${workerFileName}`, ex);
reject(ex);
});
worker.on('exit', (code) => {
traceVerbose(`Worker id ${id} exited with code ${code}`);
if (code !== 0) {
reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`));
}
});
});
}
16 changes: 16 additions & 0 deletions src/client/common/process/worker/plainExec.worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { parentPort, workerData } from 'worker_threads';
import { _workerPlainExecImpl } from './workerRawProcessApis';

_workerPlainExecImpl(workerData.file, workerData.args, workerData.options)
.then((res) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
parentPort.postMessage({ res });
})
.catch((err) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
parentPort.postMessage({ err });
});
26 changes: 26 additions & 0 deletions src/client/common/process/worker/rawProcessApiWrapper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { SpawnOptions } from 'child_process';
import * as path from 'path';
import { executeWorkerFile } from './main';
import { ExecutionResult, ShellOptions } from './types';

export function workerShellExec(command: string, options: ShellOptions): Promise<ExecutionResult<string>> {
return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), {
command,
options,
});
}

export function workerPlainExec(
file: string,
args: string[],
options: SpawnOptions = {},
): Promise<ExecutionResult<string>> {
return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), {
file,
args,
options,
});
}
16 changes: 16 additions & 0 deletions src/client/common/process/worker/shellExec.worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { parentPort, workerData } from 'worker_threads';
import { _workerShellExecImpl } from './workerRawProcessApis';

_workerShellExecImpl(workerData.command, workerData.options, workerData.defaultEnv)
.then((res) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
parentPort.postMessage({ res });
})
.catch((ex) => {
if (!parentPort) {
throw new Error('Not in a worker thread');
}
parentPort.postMessage({ ex });
});
38 changes: 38 additions & 0 deletions src/client/common/process/worker/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* eslint-disable @typescript-eslint/no-empty-function */
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
import { ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process';

export function noop() {}
export interface IDisposable {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
dispose(): void | undefined | Promise<void>;
}
export type EnvironmentVariables = Record<string, string | undefined>;
export class StdErrError extends Error {
// eslint-disable-next-line @typescript-eslint/no-useless-constructor
constructor(message: string) {
super(message);
}
}

export type SpawnOptions = ChildProcessSpawnOptions & {
encoding?: string;
// /**
// * Can't use `CancellationToken` here as it comes from vscode which is not available in worker threads.
// */
// token?: CancellationToken;
mergeStdOutErr?: boolean;
throwOnStdErr?: boolean;
extraVariables?: NodeJS.ProcessEnv;
// /**
// * Can't use `OutputChannel` here as it comes from vscode which is not available in worker threads.
// */
// outputChannel?: OutputChannel;
stdinStr?: string;
};
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };

export type ExecutionResult<T extends string | Buffer> = {
stdout: T;
stderr?: T;
};
Loading

0 comments on commit f6e1338

Please sign in to comment.