Skip to content

Remove jedi memory telemetry #15832

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

Merged
merged 1 commit into from
Apr 5, 2021
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
35 changes: 3 additions & 32 deletions package-lock.json

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

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2017,7 +2017,6 @@
"named-js-regexp": "^1.3.3",
"node-fetch": "^2.6.1",
"node-stream-zip": "^1.6.0",
"pidusage-tree": "^2.0.5",
"portfinder": "^1.0.25",
"reflect-metadata": "^0.1.12",
"request": "^2.87.0",
Expand Down
89 changes: 4 additions & 85 deletions src/client/activation/jedi/languageServerProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,18 @@ import {
import { ChildProcess } from 'child_process';
import { DeprecatePythonPath } from '../../common/experiments/groups';
import { traceDecorators, traceError } from '../../common/logger';
import {
IConfigurationService,
IExperimentsManager,
IInterpreterPathService,
IPythonSettings,
Resource,
} from '../../common/types';
import { IExperimentsManager, IInterpreterPathService, Resource } from '../../common/types';
import { swallowExceptions } from '../../common/utils/decorators';
import { LanguageServerSymbolProvider } from '../../providers/symbolProvider';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { captureTelemetry } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { ITestingService } from '../../testing/types';
import { FileBasedCancellationStrategy } from '../common/cancellationUtils';
import { LanguageClientMiddleware } from '../languageClientMiddleware';
import { ProgressReporting } from '../progress';
import { ILanguageClientFactory, ILanguageServerProxy } from '../types';
import { StopWatch } from '../../common/utils/stopWatch';
import { getMemoryUsage } from '../../common/process/memory';
import { killPidTree } from '../../common/process/rawProcessApis';
import { killPid } from '../../common/process/rawProcessApis';

@injectable()
export class JediLanguageServerProxy implements ILanguageServerProxy {
Expand All @@ -46,18 +38,11 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {

private lsVersion: string | undefined;

private pidUsageFailures = { timer: new StopWatch(), counter: 0 };

private pythonSettings: IPythonSettings | undefined;

private timer?: NodeJS.Timer | number;

constructor(
@inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory,
@inject(ITestingService) private readonly testManager: ITestingService,
@inject(IExperimentsManager) private readonly experiments: IExperimentsManager,
@inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
) {}

private static versionTelemetryProps(instance: JediLanguageServerProxy) {
Expand All @@ -73,7 +58,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
const pid: number | undefined = ((this.languageClient as any)._serverProcess as ChildProcess)?.pid;
const killServer = () => {
if (pid) {
killPidTree(pid);
killPid(pid);
}
};
// Do not await on this.
Expand All @@ -92,11 +77,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
this.cancellationStrategy = undefined;
}

if (this.timer) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
clearTimeout(this.timer as any);
}

while (this.disposables.length > 0) {
const d = this.disposables.shift()!;
d.dispose();
Expand All @@ -122,8 +102,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
return this.serverReady();
}

this.pythonSettings = this.configurationService.getSettings(resource);

this.lsVersion =
(options.middleware ? (<LanguageClientMiddleware>options.middleware).serverVersion : undefined) ?? '0.19.3';

Expand All @@ -150,13 +128,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
}

await this.registerTestServices();

try {
await this.checkJediLSPMemoryFootprint();
} catch (ex) {
// Ignore errors
}

return Promise.resolve();
}

Expand Down Expand Up @@ -209,56 +180,4 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
);
}
}

private async checkJediLSPMemoryFootprint() {
// Check memory footprint periodically. Do not check on every request due to
// the performance impact. See https://github.com/soyuka/pidusage - on Windows
// it is using wmic which means spawning cmd.exe process on every request.
if (this.pythonSettings && this.pythonSettings.jediMemoryLimit === -1) {
return;
}

await this.sendJediMemoryTelemetry();
if (this.timer) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
clearTimeout(this.timer as any);
}
this.timer = setTimeout(() => this.checkJediLSPMemoryFootprint(), 15 * 1000);
}

private async sendJediMemoryTelemetry(): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const proc: ChildProcess | undefined = (this.languageClient as any)._serverProcess as ChildProcess;
if (
!proc ||
proc.killed ||
this.pidUsageFailures.counter > 2 ||
!this.pythonSettings ||
this.pythonSettings.jediMemoryLimit === -1
) {
return;
}

try {
const memory = await getMemoryUsage(proc.pid);
const limit = Math.min(Math.max(this.pythonSettings.jediMemoryLimit, 3072), 8192) * 1024 * 1024;
if (memory > 0) {
const props = {
memUse: memory,
limit,
isUserDefinedLimit: limit !== 1024,
restart: false,
};
sendTelemetryEvent(EventName.JEDI_MEMORY, undefined, props);
}
} catch (err) {
this.pidUsageFailures.counter += 1;
// If this function fails 2 times in the last 60 seconds, lets not try ever again.
if (this.pidUsageFailures.timer.elapsedTime > 60 * 1000) {
this.pidUsageFailures.counter = 0;
this.pidUsageFailures.timer.reset();
}
traceError('Python Extension: (pidusage-tree)', err);
}
}
}
21 changes: 0 additions & 21 deletions src/client/common/process/memory.ts

This file was deleted.

18 changes: 0 additions & 18 deletions src/client/common/process/rawProcessApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import {
} from './types';
import { noop } from '../utils/misc';

const pidUsageTree = require('pidusage-tree');

function getDefaultOptions<T extends ShellOptions | SpawnOptions>(options: T, defaultEnv?: EnvironmentVariables): T {
const defaultOptions = { ...options };
const execOptions = defaultOptions as SpawnOptions;
Expand Down Expand Up @@ -253,19 +251,3 @@ export function killPid(pid: number): void {
// Ignore.
}
}

/**
* Kills all processes spawned by `pid` (`pid` inclusive)
* @param pid
*/
export async function killPidTree(pid: number): Promise<void> {
try {
// This can fail if the process is already killed
const result = await pidUsageTree(pid);
for (const key of Object.keys(result)) {
killPid(parseInt(key, 10));
}
} catch (ex) {
// Ignore.
}
}
Loading