Skip to content

Commit 68a443f

Browse files
authored
feat(amazonq): allow child processes to define custom thresholds (aws#7205)
## Problem Some processes use significantly more cpu/memory than others, so we shouldn't hold them to the same static threshold. As an example, the language servers are consistently using more memory than the 100 MB threshold. Based on some experimentation with different sized workspaces, I've noticed that the process running agentic chat uses roughly 80MB-150MB of memory, and the workspace indexing uses 550 - 700 MB memory. ## Solution - Allow each child process to set a warning threshold, with a default of 100MB of memory. - Set the Q LSP threshold to 200 MB, and workspace indexing to 800 MB. - This should help to allow these logs to point to real concerns, rather than be noise. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 91f584f commit 68a443f

File tree

5 files changed

+99
-17
lines changed

5 files changed

+99
-17
lines changed

packages/amazonq/src/lsp/client.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
isAmazonInternalOs,
3636
fs,
3737
} from 'aws-core-vscode/shared'
38+
import { processUtils } from 'aws-core-vscode/shared'
3839
import { activate } from './chat/activation'
3940
import { AmazonQResourcePaths } from './lspInstaller'
4041
import { ConfigSection, isValidConfigSection, toAmazonQLSPLogLevel } from './config'
@@ -80,11 +81,13 @@ export async function startLanguageServer(
8081
executable = [resourcePaths.node]
8182
}
8283

84+
const memoryWarnThreshold = 1024 * processUtils.oneMB
8385
const serverOptions = createServerOptions({
8486
encryptionKey,
8587
executable: executable,
8688
serverModule,
8789
execArgv: argv,
90+
warnThresholds: { memory: memoryWarnThreshold },
8891
})
8992

9093
await validateNodeExe(executable, resourcePaths.lsp, argv, logger)

packages/core/src/amazonq/lsp/lspClient.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* Licensed under the MIT License. See License.txt in the project root for license information.
99
*/
1010
import * as vscode from 'vscode'
11+
import { oneMB } from '../../shared/utilities/processUtils'
1112
import * as path from 'path'
1213
import * as nls from 'vscode-nls'
1314
import * as crypto from 'crypto'
@@ -252,13 +253,15 @@ export async function activate(extensionContext: ExtensionContext, resourcePaths
252253
}
253254

254255
const serverModule = resourcePaths.lsp
256+
const memoryWarnThreshold = 800 * oneMB
255257

256258
const serverOptions = createServerOptions({
257259
encryptionKey: key,
258260
executable: [resourcePaths.node],
259261
serverModule,
260262
// TODO(jmkeyes): we always use the debug options...?
261263
execArgv: debugOptions.execArgv,
264+
warnThresholds: { memory: memoryWarnThreshold },
262265
})
263266

264267
const documentSelector = [{ scheme: 'file', language: '*' }]

packages/core/src/shared/lsp/utils/platform.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,21 @@ export function createServerOptions({
8181
executable,
8282
serverModule,
8383
execArgv,
84+
warnThresholds,
8485
}: {
8586
encryptionKey: Buffer
8687
executable: string[]
8788
serverModule: string
8889
execArgv: string[]
90+
warnThresholds?: { cpu?: number; memory?: number }
8991
}) {
9092
return async () => {
9193
const bin = executable[0]
9294
const args = [...executable.slice(1), serverModule, ...execArgv]
9395
if (isDebugInstance()) {
9496
args.unshift('--inspect=6080')
9597
}
96-
const lspProcess = new ChildProcess(bin, args)
98+
const lspProcess = new ChildProcess(bin, args, { warnThresholds })
9799

98100
// this is a long running process, awaiting it will never resolve
99101
void lspProcess.run()

packages/core/src/shared/utilities/processUtils.ts

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ export interface ChildProcessOptions {
4444
onStdout?: (text: string, context: RunParameterContext) => void
4545
/** Callback for intercepting text from the stderr stream. */
4646
onStderr?: (text: string, context: RunParameterContext) => void
47+
/** Thresholds to configure warning logs */
48+
warnThresholds?: {
49+
/** Threshold for memory usage in bytes */
50+
memory?: number
51+
/** Threshold for CPU usage by percentage */
52+
cpu?: number
53+
}
4754
}
4855

4956
export interface ChildProcessRunOptions extends Omit<ChildProcessOptions, 'logging'> {
@@ -60,19 +67,19 @@ export interface ChildProcessResult {
6067
stderr: string
6168
signal?: string
6269
}
63-
70+
export const oneMB = 1024 * 1024
6471
export const eof = Symbol('EOF')
72+
export const defaultProcessWarnThresholds = {
73+
memory: 100 * oneMB,
74+
cpu: 50,
75+
}
6576

6677
export interface ProcessStats {
6778
memory: number
6879
cpu: number
6980
}
7081
export class ChildProcessTracker {
7182
static readonly pollingInterval: number = 10000 // Check usage every 10 seconds
72-
static readonly thresholds: ProcessStats = {
73-
memory: 100 * 1024 * 1024, // 100 MB
74-
cpu: 50,
75-
}
7683
static readonly logger = logger.getLogger('childProcess')
7784
static readonly loggedPids = new CircularBuffer(1000)
7885
#processByPid: Map<number, ChildProcess> = new Map<number, ChildProcess>()
@@ -82,6 +89,15 @@ export class ChildProcessTracker {
8289
this.#pids = new PollingSet(ChildProcessTracker.pollingInterval, () => this.monitor())
8390
}
8491

92+
private getThreshold(pid: number): ProcessStats {
93+
if (!this.#processByPid.has(pid)) {
94+
ChildProcessTracker.logOnce(pid, `Missing process with id ${pid}, returning default threshold`)
95+
return defaultProcessWarnThresholds
96+
}
97+
// Safe to assert since it exists from check above.
98+
return this.#processByPid.get(pid)!.getWarnThresholds()
99+
}
100+
85101
private cleanUp() {
86102
const terminatedProcesses = Array.from(this.#pids.values()).filter(
87103
(pid: number) => this.#processByPid.get(pid)?.stopped
@@ -106,13 +122,17 @@ export class ChildProcessTracker {
106122
return
107123
}
108124
const stats = this.getUsage(pid)
125+
const threshold = this.getThreshold(pid)
109126
if (stats) {
110127
ChildProcessTracker.logger.debug(`Process ${pid} usage: %O`, stats)
111-
if (stats.memory > ChildProcessTracker.thresholds.memory) {
112-
ChildProcessTracker.logOnce(pid, `Process ${pid} exceeded memory threshold: ${stats.memory}`)
128+
if (stats.memory > threshold.memory) {
129+
ChildProcessTracker.logOnce(
130+
pid,
131+
`Process ${pid} exceeded memory threshold: ${(stats.memory / oneMB).toFixed(2)} MB`
132+
)
113133
}
114-
if (stats.cpu > ChildProcessTracker.thresholds.cpu) {
115-
ChildProcessTracker.logOnce(pid, `Process ${pid} exceeded cpu threshold: ${stats.cpu}`)
134+
if (stats.cpu > threshold.cpu) {
135+
ChildProcessTracker.logOnce(pid, `Process ${pid} exceeded cpu threshold: ${stats.cpu}%`)
116136
}
117137
}
118138
}
@@ -248,6 +268,10 @@ export class ChildProcess {
248268
return await new ChildProcess(command, args, options).run()
249269
}
250270

271+
public getWarnThresholds(): ProcessStats {
272+
return { ...defaultProcessWarnThresholds, ...this.#baseOptions.warnThresholds }
273+
}
274+
251275
// Inspired by 'got'
252276
/**
253277
* Creates a one-off {@link ChildProcess} class that always uses the specified options.

packages/core/src/test/shared/utilities/processUtils.test.ts

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import * as sinon from 'sinon'
1010
import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../../shared/filesystemUtilities'
1111
import {
1212
ChildProcess,
13+
ChildProcessOptions,
1314
ChildProcessResult,
1415
ChildProcessTracker,
16+
defaultProcessWarnThresholds,
1517
eof,
1618
ProcessStats,
1719
} from '../../../shared/utilities/processUtils'
@@ -376,8 +378,8 @@ async function stopAndWait(runningProcess: RunningProcess): Promise<void> {
376378
await runningProcess.result
377379
}
378380

379-
function startSleepProcess(timeout: number = 90): RunningProcess {
380-
const childProcess = new ChildProcess(getSleepCmd(), [timeout.toString()])
381+
function startSleepProcess(options?: ChildProcessOptions, timeout: number = 90): RunningProcess {
382+
const childProcess = new ChildProcess(getSleepCmd(), [timeout.toString()], options)
381383
const result = childProcess.run().catch(() => assert.fail('sleep command threw an error'))
382384
return { childProcess, result }
383385
}
@@ -454,12 +456,12 @@ describe('ChildProcessTracker', function () {
454456
tracker.add(runningProcess.childProcess)
455457

456458
const highCpu: ProcessStats = {
457-
cpu: ChildProcessTracker.thresholds.cpu + 1,
459+
cpu: defaultProcessWarnThresholds.cpu + 1,
458460
memory: 0,
459461
}
460462
const highMemory: ProcessStats = {
461463
cpu: 0,
462-
memory: ChildProcessTracker.thresholds.memory + 1,
464+
memory: defaultProcessWarnThresholds.memory + 1,
463465
}
464466

465467
usageMock.returns(highCpu)
@@ -480,7 +482,7 @@ describe('ChildProcessTracker', function () {
480482
tracker.add(runningProcess.childProcess)
481483

482484
usageMock.returns({
483-
cpu: ChildProcessTracker.thresholds.cpu + 1,
485+
cpu: defaultProcessWarnThresholds.cpu + 1,
484486
memory: 0,
485487
})
486488

@@ -492,10 +494,11 @@ describe('ChildProcessTracker', function () {
492494

493495
it('does not log for processes within threshold', async function () {
494496
const runningProcess = startSleepProcess()
497+
tracker.add(runningProcess.childProcess)
495498

496499
usageMock.returns({
497-
cpu: ChildProcessTracker.thresholds.cpu - 1,
498-
memory: ChildProcessTracker.thresholds.memory - 1,
500+
cpu: defaultProcessWarnThresholds.cpu - 1,
501+
memory: defaultProcessWarnThresholds.memory - 1,
499502
})
500503

501504
await clock.tickAsync(ChildProcessTracker.pollingInterval)
@@ -504,4 +507,51 @@ describe('ChildProcessTracker', function () {
504507

505508
await stopAndWait(runningProcess)
506509
})
510+
511+
it('respects custom thresholds', async function () {
512+
const largeRunningProcess = startSleepProcess({
513+
warnThresholds: {
514+
cpu: defaultProcessWarnThresholds.cpu + 10,
515+
memory: defaultProcessWarnThresholds.memory + 10,
516+
},
517+
})
518+
tracker.add(largeRunningProcess.childProcess)
519+
const smallRunningProcess = startSleepProcess({
520+
warnThresholds: {
521+
cpu: defaultProcessWarnThresholds.cpu - 10,
522+
memory: defaultProcessWarnThresholds.memory - 10,
523+
},
524+
})
525+
tracker.add(smallRunningProcess.childProcess)
526+
527+
usageMock.returns({
528+
cpu: defaultProcessWarnThresholds.cpu + 5,
529+
memory: defaultProcessWarnThresholds.memory + 5,
530+
})
531+
532+
await clock.tickAsync(ChildProcessTracker.pollingInterval)
533+
assert.throws(() => assertLogsContain(largeRunningProcess.childProcess.pid().toString(), false, 'warn'))
534+
assertLogsContain(smallRunningProcess.childProcess.pid().toString(), false, 'warn')
535+
536+
await stopAndWait(largeRunningProcess)
537+
await stopAndWait(smallRunningProcess)
538+
})
539+
540+
it('fills custom thresholds with default', async function () {
541+
const runningProcess = startSleepProcess({
542+
warnThresholds: {
543+
cpu: defaultProcessWarnThresholds.cpu + 10,
544+
},
545+
})
546+
tracker.add(runningProcess.childProcess)
547+
548+
usageMock.returns({
549+
memory: defaultProcessWarnThresholds.memory + 1,
550+
})
551+
552+
await clock.tickAsync(ChildProcessTracker.pollingInterval)
553+
assertLogsContain(runningProcess.childProcess.pid().toString(), false, 'warn')
554+
555+
await stopAndWait(runningProcess)
556+
})
507557
})

0 commit comments

Comments
 (0)