Skip to content

Commit b31ec28

Browse files
Mossakaclaude
andauthored
fix: measure memory while containers are running, not after teardown (#1765)
* fix: measure memory while containers are running, not after teardown Previously benchmarkMemory() ran `echo measuring_memory` which completed instantly, so containers were already stopped before docker stats ran, always reporting 0 MB. Now spawns awf with `sleep 30` in the background, polls until containers are healthy, then samples docker stats while containers are alive. Closes #1758 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: exact name matching, process group cleanup - Use anchored regex (^name$) in docker ps filter to avoid substring false positives from similarly-named containers - Send SIGKILL to the entire process group (not just the direct child) so descendant processes like sudo/awf/docker don't survive - Add child.unref() to prevent parent from hanging if cleanup fails - Derive spawn args from AWF_CMD constant to avoid drift Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 990682f commit b31ec28

File tree

1 file changed

+108
-23
lines changed

1 file changed

+108
-23
lines changed

scripts/ci/benchmark-performance.ts

Lines changed: 108 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* - stderr (console.error): progress messages and diagnostics — kept separate so JSON stays valid
1616
*/
1717

18-
import { execSync, ExecSyncOptions } from "child_process";
18+
import { execSync, ExecSyncOptions, spawn, ChildProcess } from "child_process";
1919

2020
// ── Configuration ──────────────────────────────────────────────────
2121

@@ -163,45 +163,130 @@ function benchmarkHttpsLatency(): BenchmarkResult {
163163
return { metric: "squid_https_latency", unit: "ms", values, ...stats(values) };
164164
}
165165

166-
function benchmarkMemory(): BenchmarkResult {
166+
/**
167+
* Wait for Docker containers to be running, polling at 500ms intervals.
168+
* Uses exact name matching (anchored regex) to avoid false positives from
169+
* containers with similar names (e.g., "awf-squid-old").
170+
* Throws if containers are not running within timeoutMs.
171+
*/
172+
function waitForContainers(containerNames: string[], timeoutMs: number): Promise<void> {
173+
const start = Date.now();
174+
return new Promise((resolve, reject) => {
175+
const poll = (): void => {
176+
if (Date.now() - start > timeoutMs) {
177+
reject(new Error(`Containers not running after ${timeoutMs}ms`));
178+
return;
179+
}
180+
try {
181+
const allRunning = containerNames.every((name) => {
182+
const result = execSync(
183+
`sudo docker ps --filter name=^${name}$ --filter status=running --format '{{.Names}}' 2>/dev/null`,
184+
{ encoding: "utf-8", timeout: 5_000 }
185+
)
186+
.trim()
187+
.split("\n")
188+
.map((n) => n.trim())
189+
.filter(Boolean);
190+
return result.some((n) => n === name);
191+
});
192+
if (allRunning) {
193+
resolve();
194+
return;
195+
}
196+
} catch {
197+
// container not ready yet
198+
}
199+
setTimeout(poll, 500);
200+
};
201+
poll();
202+
});
203+
}
204+
205+
/**
206+
* Parse a Docker memory usage string like "123.4MiB / 7.773GiB" into MB.
207+
*/
208+
function parseMb(s: string): number {
209+
const match = s.match(/([\d.]+)\s*(MiB|GiB|KiB)/i);
210+
if (!match) return 0;
211+
const val = parseFloat(match[1]);
212+
const unit = match[2].toLowerCase();
213+
if (unit === "gib") return val * 1024;
214+
if (unit === "kib") return val / 1024;
215+
return val;
216+
}
217+
218+
/**
219+
* Kill a spawned background process and its entire process group, best-effort.
220+
* Sends SIGTERM then SIGKILL to the process group so descendant processes
221+
* (e.g., sudo, awf, docker) don't survive.
222+
*/
223+
function killBackground(child: ChildProcess): void {
224+
const pid = child.pid;
225+
if (!pid) return;
226+
227+
try {
228+
// SIGTERM the process group to allow graceful shutdown
229+
process.kill(-pid, "SIGTERM");
230+
} catch {
231+
// Process group may have already exited
232+
}
233+
234+
try {
235+
// SIGKILL the entire process group to ensure nothing survives
236+
process.kill(-pid, "SIGKILL");
237+
} catch {
238+
// Process group may have already exited
239+
}
240+
}
241+
242+
async function benchmarkMemory(): Promise<BenchmarkResult> {
167243
console.error(" Benchmarking memory footprint...");
168244
const values: number[] = [];
169245

170246
for (let i = 0; i < ITERATIONS; i++) {
171247
cleanup();
172-
// Start containers, measure memory, then stop
248+
let child: ChildProcess | null = null;
173249
try {
174-
// Run a sleep command so containers stay up, then check memory
175-
const output = exec(
176-
`${AWF_CMD} --allow-domains ${ALLOWED_DOMAIN} --log-level error --keep-containers -- ` +
177-
`echo measuring_memory`
250+
// Start awf with a long-running command in the background so containers stay alive.
251+
// Derive spawn args from AWF_CMD to stay consistent with the rest of the script.
252+
const awfParts = AWF_CMD.split(/\s+/);
253+
child = spawn(
254+
awfParts[0],
255+
[...awfParts.slice(1), "--allow-domains", ALLOWED_DOMAIN, "--log-level", "error", "--", "sleep", "30"],
256+
{
257+
detached: true,
258+
stdio: "ignore",
259+
}
178260
);
179-
// Get memory stats for both containers
261+
// Unref so the parent process won't be kept alive if cleanup fails
262+
child.unref();
263+
264+
// Wait for both containers to be running (up to 30s)
265+
await waitForContainers(["awf-squid", "awf-agent"], 30_000);
266+
267+
// Give containers a moment to stabilize memory usage
268+
await new Promise((resolve) => setTimeout(resolve, 2000));
269+
270+
// Get memory stats while containers are alive
180271
const squidMem = exec(
181272
"sudo docker stats awf-squid --no-stream --format '{{.MemUsage}}' 2>/dev/null || echo '0MiB'"
182273
);
183274
const agentMem = exec(
184275
"sudo docker stats awf-agent --no-stream --format '{{.MemUsage}}' 2>/dev/null || echo '0MiB'"
185276
);
186277

187-
// Parse memory values (format: "123.4MiB / 7.773GiB")
188-
const parseMb = (s: string): number => {
189-
const match = s.match(/([\d.]+)\s*(MiB|GiB|KiB)/i);
190-
if (!match) return 0;
191-
const val = parseFloat(match[1]);
192-
const unit = match[2].toLowerCase();
193-
if (unit === "gib") return val * 1024;
194-
if (unit === "kib") return val / 1024;
195-
return val;
196-
};
197-
198278
const totalMb = Math.round(parseMb(squidMem) + parseMb(agentMem));
199279
values.push(totalMb);
200280
console.error(` Iteration ${i + 1}/${ITERATIONS}: ${totalMb}MB (squid: ${squidMem}, agent: ${agentMem})`);
201-
} catch {
202-
console.error(` Iteration ${i + 1}/${ITERATIONS}: failed (skipped)`);
281+
} catch (err) {
282+
console.error(` Iteration ${i + 1}/${ITERATIONS}: failed (skipped) - ${err}`);
283+
} finally {
284+
// Always clean up the background process and containers
285+
if (child) {
286+
killBackground(child);
287+
}
288+
cleanup();
203289
}
204-
cleanup();
205290
}
206291

207292
if (values.length === 0) {
@@ -252,7 +337,7 @@ async function main(): Promise<void> {
252337
results.push(benchmarkWarmStart());
253338
results.push(benchmarkColdStart());
254339
results.push(benchmarkHttpsLatency());
255-
results.push(benchmarkMemory());
340+
results.push(await benchmarkMemory());
256341

257342
// Final cleanup
258343
cleanup();

0 commit comments

Comments
 (0)