Skip to content

perf(security): Reduce security worker threads to minimize CPU contention #1369

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.

Sign up for GitHub

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

Closed
yamadashy wants to merge 1 commit into from
+28 −2
Closed

perf(security): Reduce security worker threads to minimize CPU contention #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/core/security/securityCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ export const runSecurityCheck = async (
}
}

// Cap security worker pool to 1 thread to minimize CPU contention with the metrics
// worker pool. Security runs concurrently with file processing and completes well before
// metrics calculation, so extra threads only compete for CPU without improving throughput.
const taskRunner = deps.initTaskRunner<SecurityCheckTask, SuspiciousFileResult | null>({
numOfTasks: rawFiles.length + gitDiffTasks.length + gitLogTasks.length,
workerType: 'securityCheck',
runtime: 'worker_threads',
maxThreads: 1,
});
const fileTasks = rawFiles.map(
(file) =>
Expand Down
6 changes: 4 additions & 2 deletions src/shared/processConcurrency.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface WorkerOptions {
numOfTasks: number;
workerType: WorkerType;
runtime: WorkerRuntime;
maxThreads?: number;
}

/**
Expand Down Expand Up @@ -62,8 +63,9 @@ export const getWorkerThreadCount = (numOfTasks: number): { minThreads: number;
};

export const createWorkerPool = (options: WorkerOptions): Tinypool => {
const { numOfTasks, workerType, runtime = 'child_process' } = options;
const { minThreads, maxThreads } = getWorkerThreadCount(numOfTasks);
const { numOfTasks, workerType, runtime = 'child_process', maxThreads: maxThreadsOverride } = options;
const { minThreads, maxThreads: autoMaxThreads } = getWorkerThreadCount(numOfTasks);
const maxThreads = maxThreadsOverride != null ? Math.max(1, maxThreadsOverride) : autoMaxThreads;
Comment on lines +67 to +68
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When maxThreadsOverride is used to set a low thread count (e.g., 1), there is a risk that minThreads (returned by getWorkerThreadCount) could exceed it, which would cause Tinypool to throw an error (as it requires minThreads <= maxThreads). Although minThreads is currently hardcoded to 1 in getWorkerThreadCount, ensuring this relationship makes the code more robust against future changes in the thread calculation logic.

Suggested change
const { minThreads, maxThreads: autoMaxThreads } = getWorkerThreadCount(numOfTasks);
const maxThreads = maxThreadsOverride != null ? Math.max(1, maxThreadsOverride) : autoMaxThreads;
const { minThreads: autoMinThreads, maxThreads: autoMaxThreads } = getWorkerThreadCount(numOfTasks);
const maxThreads = maxThreadsOverride != null ? Math.max(1, maxThreadsOverride) : autoMaxThreads;
const minThreads = Math.min(autoMinThreads, maxThreads);


// Get worker path - uses unified worker in bundled env, individual files otherwise
const workerPath = getWorkerPath(workerType);
Expand Down
20 changes: 20 additions & 0 deletions tests/shared/processConcurrency.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,26 @@ describe('processConcurrency', () => {
});
expect(tinypool).toBeDefined();
});

it('should respect maxThreads override when provided', () => {
createWorkerPool({ numOfTasks: 500, workerType: 'securityCheck', runtime: 'worker_threads', maxThreads: 1 });

expect(Tinypool).toHaveBeenCalledWith(
expect.objectContaining({
maxThreads: 1,
}),
);
});

it('should clamp maxThreads override to minimum of 1', () => {
createWorkerPool({ numOfTasks: 500, workerType: 'securityCheck', runtime: 'worker_threads', maxThreads: 0 });

expect(Tinypool).toHaveBeenCalledWith(
expect.objectContaining({
maxThreads: 1,
}),
);
});
});

describe('initTaskRunner', () => {
Expand Down
Loading