mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(bench): make bench:idle-cpu build, sample CPU and parse the process table on Windows (#27622)
* fix(bench): make the idle-CPU benchmark run and report CPU on Windows Three independent Windows defects in the idle-CPU benchmark (its report is what Windows performance work would cite): - `buildAppIfNeeded` spawned `npx`, which is `npx.cmd` on Windows, so execFileSync failed with ENOENT before any sampling. Build through `config/scripts/run-electron-vite-build.mjs` instead — the repo's own Windows-safe wrapper, which also runs the boot-graph verification. - `readWindowsProcesses` returned `cpuTimeSeconds: null`, so the sampler's per-process CPU delta (idle-cpu-process-sampling.mjs:198) fell back to the hardcoded `percentCpu: 0` and every CPU KPI in the report was zero. Read `UserModeTime`/`KernelModeTime` from `Win32_Process` (100 ns units) and keep `null` when a host does not return them. Measured on Windows, same options before and after the CPU fix: | class | mean CPU before | mean CPU after | | --- | ---: | ---: | | renderer | 0 | 2.20 | | gpu | 0 | 2.08 | | utility | 0 | 0.11 | | agent-or-node | 0 | 0.06 | | total | 0 | 4.51 (p95 8.40) | RSS is unchanged (934 MiB -> 937 MiB), so only the CPU reporting changed. Refs #25235 * fix(bench): reject a missing Win32_Process time instead of reading it as zero Review follow-up (CodeRabbit). `Number(null)` is `0`, which is also a real counter value, so an absent `UserModeTime`/`KernelModeTime` passed the finite check and the sampler would report a kernel-only delta instead of taking the fallback. Require an actual number from the CIM payload. Verified on Windows that the real payload still reports CPU once the check is tight: total mean 4.40%, p95 14.48%, renderer 3.51%. * test(bench): cover the Windows CIM CPU time mapping The mapping was only reachable through Get-CimInstance, so no test ran it. Lift it into cimCpuSeconds and assert the 100ns-to-seconds sum plus the missing / string / non-finite cases that must stay null. * fix(bench): read the Windows process table as UTF-8 so JSON.parse survives PowerShell writes to a redirected stdout in the OEM code page (437 here), which best-fits characters such as U+2022 and U+2192 in any running process's command line to raw 0x07 / 0x1A bytes. ConvertTo-Json had left them unescaped because they are not control characters in UTF-16, so the sampler's JSON.parse failed with "Bad control character in string literal" whenever such a process happened to be alive -- hence intermittent. Set [Console]::OutputEncoding to UTF-8, as the app's own CIM scan does, and cover it with a Windows-only test that fails without the fix. Fixes #25236 * fix(bench): pass the local runtime to fetchRepos in the idle-CPU fixture fetchRepos now requires a runtimeEnvironmentId, so the fixture's bare call threw "Cannot read properties of undefined (reading 'runtimeEnvironmentId')" before sampling on every platform. null selects this app's own catalog, as the e2e helpers already do. * ci(bench): run the Windows process-table test in the Windows lane The UTF-8 regression test self-skips off win32, so it must be registered in both Windows lists or it runs on no machine. --------- Co-authored-by: Paolo <paovalletta@hotmail.it> Co-authored-by: Orca Worker <orca-worker@localhost>
This commit is contained in:
co-authored by
Paolo
Orca Worker
parent
bb12a8a1e4
commit
983532a5cb
@@ -1236,6 +1236,7 @@ jobs:
|
||||
run: >-
|
||||
pnpm exec vitest run --config config/vitest.config.ts
|
||||
config/scripts/nsis-process-check.test.mjs
|
||||
config/scripts/idle-cpu-process-sampling.test.mjs
|
||||
|
||||
- name: Test Windows-specific boundaries
|
||||
run: >-
|
||||
|
||||
@@ -55,9 +55,26 @@ function readUnixProcesses() {
|
||||
return parseUnixProcesses(stdout)
|
||||
}
|
||||
|
||||
// `Win32_Process` times are 100 ns counts. Not `Number(value)`: that turns a missing
|
||||
// field into a real-looking 0, so the CPU delta would silently go kernel-only.
|
||||
function cimTimeSeconds(value) {
|
||||
return typeof value === 'number' && Number.isFinite(value) ? value / 10_000_000 : null
|
||||
}
|
||||
|
||||
export function cimCpuSeconds(entry) {
|
||||
const userModeSeconds = cimTimeSeconds(entry.UserModeTime)
|
||||
const kernelModeSeconds = cimTimeSeconds(entry.KernelModeTime)
|
||||
return userModeSeconds !== null && kernelModeSeconds !== null
|
||||
? userModeSeconds + kernelModeSeconds
|
||||
: null
|
||||
}
|
||||
|
||||
function readWindowsProcesses() {
|
||||
// Why UTF-8: the default OEM code page best-fits characters such as `•` or `→` in any
|
||||
// process's command line to raw control bytes (0x07, 0x1A), which JSON.parse rejects.
|
||||
const script =
|
||||
'Get-CimInstance Win32_Process | Select-Object ProcessId,ParentProcessId,WorkingSetSize,CommandLine | ConvertTo-Json -Compress'
|
||||
'[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; ' +
|
||||
'Get-CimInstance Win32_Process | Select-Object ProcessId,ParentProcessId,WorkingSetSize,UserModeTime,KernelModeTime,CommandLine | ConvertTo-Json -Compress'
|
||||
const result = spawnSync('powershell.exe', ['-NoProfile', '-Command', script], {
|
||||
encoding: 'utf8',
|
||||
maxBuffer: 20 * 1024 * 1024
|
||||
@@ -71,7 +88,7 @@ function readWindowsProcesses() {
|
||||
pid: Number(entry.ProcessId),
|
||||
ppid: Number(entry.ParentProcessId),
|
||||
percentCpu: 0,
|
||||
cpuTimeSeconds: null,
|
||||
cpuTimeSeconds: cimCpuSeconds(entry),
|
||||
rssBytes: Number(entry.WorkingSetSize) || 0,
|
||||
command: String(entry.CommandLine || '')
|
||||
}))
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { spawn } from 'node:child_process'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { sampleProcessTreeUntilWorkloadsComplete } from './idle-cpu-process-sampling.mjs'
|
||||
import {
|
||||
cimCpuSeconds,
|
||||
readProcessRows,
|
||||
sampleProcessTreeUntilWorkloadsComplete
|
||||
} from './idle-cpu-process-sampling.mjs'
|
||||
|
||||
function createClock(workloadCompletesAt) {
|
||||
let currentMs = 0
|
||||
@@ -27,6 +32,39 @@ function createClock(workloadCompletesAt) {
|
||||
return { now: () => currentMs, readRows, wait, workloadPromise }
|
||||
}
|
||||
|
||||
describe('Windows CIM CPU time mapping', () => {
|
||||
it('converts the 100ns user and kernel times to a summed second count', () => {
|
||||
// 10_000_000 and 25_000_000 units of 100ns are 1s and 2.5s.
|
||||
expect(cimCpuSeconds({ UserModeTime: 10_000_000, KernelModeTime: 25_000_000 })).toBe(3.5)
|
||||
})
|
||||
|
||||
it('maps a missing or non-numeric time to null instead of a kernel-only reading', () => {
|
||||
expect(cimCpuSeconds({ UserModeTime: null, KernelModeTime: 10_000_000 })).toBeNull()
|
||||
expect(cimCpuSeconds({ KernelModeTime: 10_000_000 })).toBeNull()
|
||||
expect(cimCpuSeconds({ UserModeTime: '10000000', KernelModeTime: 10_000_000 })).toBeNull()
|
||||
expect(cimCpuSeconds({ UserModeTime: Number.NaN, KernelModeTime: 10_000_000 })).toBeNull()
|
||||
expect(
|
||||
cimCpuSeconds({ UserModeTime: Number.POSITIVE_INFINITY, KernelModeTime: 10_000_000 })
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe.skipIf(process.platform !== 'win32')('Windows process table read', () => {
|
||||
it('parses a command line whose characters the OEM code page maps to control bytes', () => {
|
||||
const marker = 'idle-cpu-oem • → ♪'
|
||||
const child = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 30_000)', marker], {
|
||||
stdio: 'ignore'
|
||||
})
|
||||
try {
|
||||
const row = readProcessRows().find((candidate) => candidate.pid === child.pid)
|
||||
expect(row?.command).toContain(marker)
|
||||
expect(row?.cpuTimeSeconds).toEqual(expect.any(Number))
|
||||
} finally {
|
||||
child.kill()
|
||||
}
|
||||
}, 30_000)
|
||||
})
|
||||
|
||||
describe('idle CPU process sampling window', () => {
|
||||
it('extends through a slow workload and captures a final CPU delta', async () => {
|
||||
const clock = createClock(40)
|
||||
|
||||
@@ -372,6 +372,7 @@ const WINDOWS_PACKAGE_TESTS = [
|
||||
'config/scripts/rebuild-native-deps-windows-process-tree.test.mjs',
|
||||
'config/scripts/rebuild-native-deps-node-pty.test.mjs',
|
||||
'config/scripts/nsis-process-check.test.mjs',
|
||||
'config/scripts/idle-cpu-process-sampling.test.mjs',
|
||||
'config/scripts/ensure-native-runtime-job-ownership.test.mjs',
|
||||
'config/scripts/verify-packaged-node-pty-job-ownership.test.mjs',
|
||||
'config/scripts/windows-pe-machine.test.mjs',
|
||||
|
||||
@@ -159,11 +159,17 @@ function buildAppIfNeeded(root, skipBuild) {
|
||||
throw new Error(`--skip-build requested, but ${mainPath} does not exist`)
|
||||
}
|
||||
console.log('[idle-cpu] building Electron app with electron-vite --mode e2e')
|
||||
run('npx', ['electron-vite', 'build', '--mode', 'e2e'], {
|
||||
cwd: root,
|
||||
stdio: 'inherit',
|
||||
env: { ...process.env, VITE_EXPOSE_STORE: 'true' }
|
||||
})
|
||||
// Why through the wrapper: `npx` only resolves to an executable on POSIX; on Windows it is
|
||||
// `npx.cmd`, which execFileSync cannot run, and the wrapper also verifies the boot graph.
|
||||
run(
|
||||
process.execPath,
|
||||
[path.join(root, 'config', 'scripts', 'run-electron-vite-build.mjs'), '--mode', 'e2e'],
|
||||
{
|
||||
cwd: root,
|
||||
stdio: 'inherit',
|
||||
env: { ...process.env, VITE_EXPOSE_STORE: 'true' }
|
||||
}
|
||||
)
|
||||
return mainPath
|
||||
}
|
||||
|
||||
@@ -348,7 +354,7 @@ async function main() {
|
||||
return { error: added.error }
|
||||
}
|
||||
const store = window.__store
|
||||
await store?.getState().fetchRepos()
|
||||
await store?.getState().fetchRepos({ runtimeEnvironmentId: null })
|
||||
const repo = store?.getState().repos.find((candidate) => candidate.id === added.repo.id)
|
||||
if (repo) {
|
||||
const detected = await store
|
||||
|
||||
Reference in New Issue
Block a user