Skip to content

Commit

Permalink
Change worker to return interpreter information instead
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj committed Sep 21, 2020
1 parent d31f823 commit b76660f
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 63 deletions.
6 changes: 3 additions & 3 deletions src/client/pythonEnvironments/base/info/interpreter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Architecture } from '../../../common/utils/platform';
import { copyPythonExecInfo, PythonExecInfo } from '../../exec';
import { parseVersion } from './pythonVersion';

type PythonEnvInformation = {
export type InterpreterInformation = {
arch: Architecture;
executable: {
filename: string;
Expand All @@ -26,7 +26,7 @@ type PythonEnvInformation = {
* @param python - the path to the Python executable
* @param raw - the information returned by the `interpreterInfo.py` script
*/
export function extractPythonEnvInfo(python: string, raw: PythonEnvInfo): PythonEnvInformation {
export function extractPythonEnvInfo(python: string, raw: PythonEnvInfo): InterpreterInformation {
const rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}-${raw.versionInfo[3]}`;
const version = parseVersion(rawVersion);
version.sysVersion = raw.sysVersion;
Expand Down Expand Up @@ -64,7 +64,7 @@ export async function getInterpreterInfo(
python: PythonExecInfo,
shellExec: ShellExecFunc,
logger?: Logger,
): Promise<PythonEnvInformation | undefined> {
): Promise<InterpreterInformation | undefined> {
const [args, parse] = getInterpreterInfoCommand();
const info = copyPythonExecInfo(python, args);
const argv = [info.command, ...info.args];
Expand Down
42 changes: 17 additions & 25 deletions src/client/pythonEnvironments/info/environmentInfoService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
import { injectable } from 'inversify';
import { createDeferred, Deferred } from '../../common/utils/async';
import { createWorkerPool, IWorkerPool, QueuePosition } from '../../common/utils/workerPool';
import { PythonEnvInfo } from '../base/info';
import { getInterpreterInfo } from '../base/info/interpreter';
import { getInterpreterInfo, InterpreterInformation } from '../base/info/interpreter';
import { shellExecute } from '../common/externalDependencies';
import { buildPythonExecInfo } from '../exec';

Expand All @@ -17,26 +16,17 @@ export enum EnvironmentInfoServiceQueuePriority {
export const IEnvironmentInfoService = Symbol('IEnvironmentInfoService');
export interface IEnvironmentInfoService {
getEnvironmentInfo(
environment: PythonEnvInfo,
interpreterPath: string,
priority?: EnvironmentInfoServiceQueuePriority
): Promise<PythonEnvInfo | undefined>;
): Promise<InterpreterInformation | undefined>;
}

async function buildEnvironmentInfo(environment: PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
const interpreterInfo = await getInterpreterInfo(
buildPythonExecInfo(environment.executable.filename),
shellExecute,
);
async function buildEnvironmentInfo(interpreterPath: string): Promise<InterpreterInformation | undefined> {
const interpreterInfo = await getInterpreterInfo(buildPythonExecInfo(interpreterPath), shellExecute);
if (interpreterInfo === undefined || interpreterInfo.version === undefined) {
return undefined;
}
// Deep copy into a new object
const resolvedEnv = JSON.parse(JSON.stringify(environment)) as PythonEnvInfo;
resolvedEnv.version = interpreterInfo.version;
resolvedEnv.executable.filename = interpreterInfo.executable.filename;
resolvedEnv.executable.sysPrefix = interpreterInfo.executable.sysPrefix;
resolvedEnv.arch = interpreterInfo.arch;
return resolvedEnv;
return interpreterInfo;
}

@injectable()
Expand All @@ -45,29 +35,31 @@ export class EnvironmentInfoService implements IEnvironmentInfoService {
// path again and again in a given session. This information will likely not change in a given
// session. There are definitely cases where this will change. But a simple reload should address
// those.
private readonly cache: Map<string, Deferred<PythonEnvInfo>> = new Map<string, Deferred<PythonEnvInfo>>();
private readonly cache: Map<string, Deferred<InterpreterInformation>> = new Map<
string,
Deferred<InterpreterInformation>
>();

private readonly workerPool: IWorkerPool<PythonEnvInfo, PythonEnvInfo | undefined>;
private readonly workerPool: IWorkerPool<string, InterpreterInformation | undefined>;

public constructor() {
this.workerPool = createWorkerPool<PythonEnvInfo, PythonEnvInfo | undefined>(buildEnvironmentInfo);
this.workerPool = createWorkerPool<string, InterpreterInformation | undefined>(buildEnvironmentInfo);
}

public async getEnvironmentInfo(
environment: PythonEnvInfo,
interpreterPath: string,
priority?: EnvironmentInfoServiceQueuePriority,
): Promise<PythonEnvInfo | undefined> {
const interpreterPath = environment.executable.filename;
): Promise<InterpreterInformation | undefined> {
const result = this.cache.get(interpreterPath);
if (result !== undefined) {
// Another call for this environment has already been made, return its result
return result.promise;
}
const deferred = createDeferred<PythonEnvInfo>();
const deferred = createDeferred<InterpreterInformation>();
this.cache.set(interpreterPath, deferred);
return (priority === EnvironmentInfoServiceQueuePriority.High
? this.workerPool.addToQueue(environment, QueuePosition.Front)
: this.workerPool.addToQueue(environment, QueuePosition.Back)
? this.workerPool.addToQueue(interpreterPath, QueuePosition.Front)
: this.workerPool.addToQueue(interpreterPath, QueuePosition.Back)
).then((r) => {
if (r !== undefined) {
deferred.resolve(r);
Expand Down
45 changes: 10 additions & 35 deletions src/test/pythonEnvironments/info/environmentInfoService.functional.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as sinon from 'sinon';
import { ImportMock } from 'ts-mock-imports';
import { ExecutionResult } from '../../../client/common/process/types';
import { Architecture } from '../../../client/common/utils/platform';
import { PythonEnvInfo, PythonEnvKind } from '../../../client/pythonEnvironments/base/info';
import { InterpreterInformation } from '../../../client/pythonEnvironments/base/info/interpreter';
import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion';
import * as ExternalDep from '../../../client/pythonEnvironments/common/externalDependencies';
import {
Expand All @@ -19,39 +19,16 @@ import {
suite('Environment Info Service', () => {
let stubShellExec: sinon.SinonStub;

function createEnvInfo(executable: string): PythonEnvInfo {
function createExpectedEnvInfo(executable: string): InterpreterInformation {
return {
id: '',
kind: PythonEnvKind.Unknown,
version: parseVersion('0.0.0'),
name: '',
location: '',
arch: Architecture.x64,
executable: {
filename: executable,
sysPrefix: '',
mtime: -1,
ctime: -1,
},
distro: { org: '' },
};
}

function createExpectedEnvInfo(executable: string): PythonEnvInfo {
return {
id: '',
kind: PythonEnvKind.Unknown,
version: parseVersion('3.8.3-final'),
name: '',
location: '',
arch: Architecture.x64,
executable: {
filename: executable,
sysPrefix: 'path',
mtime: -1,
ctime: -1,
},
distro: { org: '' },
};
}

Expand All @@ -72,16 +49,14 @@ suite('Environment Info Service', () => {
});
test('Add items to queue and get results', async () => {
const envService = new EnvironmentInfoService();
const promises: Promise<PythonEnvInfo | undefined>[] = [];
const expected: PythonEnvInfo[] = [];
const promises: Promise<InterpreterInformation | undefined>[] = [];
const expected: InterpreterInformation[] = [];
for (let i = 0; i < 10; i = i + 1) {
const path = `any-path${i}`;
if (i < 5) {
promises.push(envService.getEnvironmentInfo(createEnvInfo(path)));
promises.push(envService.getEnvironmentInfo(path));
} else {
promises.push(
envService.getEnvironmentInfo(createEnvInfo(path), EnvironmentInfoServiceQueuePriority.High),
);
promises.push(envService.getEnvironmentInfo(path, EnvironmentInfoServiceQueuePriority.High));
}
expected.push(createExpectedEnvInfo(path));
}
Expand All @@ -97,17 +72,17 @@ suite('Environment Info Service', () => {

test('Add same item to queue', async () => {
const envService = new EnvironmentInfoService();
const promises: Promise<PythonEnvInfo | undefined>[] = [];
const expected: PythonEnvInfo[] = [];
const promises: Promise<InterpreterInformation | undefined>[] = [];
const expected: InterpreterInformation[] = [];

const path = 'any-path';
// Clear call counts
stubShellExec.resetHistory();
// Evaluate once so the result is cached.
await envService.getEnvironmentInfo(createEnvInfo(path));
await envService.getEnvironmentInfo(path);

for (let i = 0; i < 10; i = i + 1) {
promises.push(envService.getEnvironmentInfo(createEnvInfo(path)));
promises.push(envService.getEnvironmentInfo(path));
expected.push(createExpectedEnvInfo(path));
}

Expand Down

0 comments on commit b76660f

Please sign in to comment.