Skip to content

Commit 4d14ff4

Browse files
committed
fix(bench): document p95 in BMF schema; clean up runner.py fragment files
Per Gemini review on PR #21. 1. README.md schema example listed only latency_p50_ns + latency_p99_ns, but bench_latency.nim emits all three (p50/p95/p99). Added p95 to the schema block and the prose paragraph below it. The Metrics section already correctly listed p50/p95/p99. 2. runner.py left N timestamped fragment files in RESULTS_DIR after each merged run (one per Nim binary) — only the merged file is the canonical artifact. Wrap the run+merge in try/finally and unlink each fragment after, so a bench-binary failure or a merge collision still cleans up whatever was produced.
1 parent def6f54 commit 4d14ff4

2 files changed

Lines changed: 33 additions & 22 deletions

File tree

benchmarks/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ job log so local debugging is possible without the upload.
116116
"upper_value": <mean + stddev>
117117
},
118118
"latency_p50_ns": {"value": <ns>},
119+
"latency_p95_ns": {"value": <ns>},
119120
"latency_p99_ns": {"value": <ns>}
120121
}
121122
}
@@ -126,8 +127,9 @@ within each slug. `lower_value` / `upper_value` are omitted when the
126127
emitter receives `NaN` sentinels for the bounds. After `merge_bmf.py`
127128
unions the five binary fragments, a single slug can carry both
128129
`throughput_ops_ms` (from the matching topology binary) AND
129-
`latency_p50_ns` / `latency_p99_ns` (from `bench_latency`) when the
130-
slug shape matches `1p1c` on a bounded variant.
130+
`latency_p50_ns` / `latency_p95_ns` / `latency_p99_ns` (from
131+
`bench_latency`) when the slug shape matches `1p1c` on a bounded
132+
variant.
131133

132134
Current slug set emitted across the five binaries:
133135

benchmarks/runner.py

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,38 @@ def run_nim(runs: int, output_file: Path):
6161
"""
6262
print(f"Running Nim topology-split benchmarks ({runs} runs)...")
6363
fragments: list[Path] = []
64-
for bin_name in NIM_BINARIES:
65-
bin_path = PROJECT_ROOT / ".tmp" / bin_name
66-
out = output_file.parent / f"{output_file.stem}-{bin_name}.json"
67-
print(f" -> {bin_name} -> {out.name}")
64+
try:
65+
for bin_name in NIM_BINARIES:
66+
bin_path = PROJECT_ROOT / ".tmp" / bin_name
67+
out = output_file.parent / f"{output_file.stem}-{bin_name}.json"
68+
print(f" -> {bin_name} -> {out.name}")
69+
subprocess.run(
70+
[str(bin_path), f"--bmf-out={out}"],
71+
check=True,
72+
)
73+
fragments.append(out)
74+
# Merge the 5 BMF fragments into the requested output file via
75+
# merge_bmf.py. Unions per-slug measure dicts and exits 1 on
76+
# (slug, measure) collisions.
77+
print(f"Merging {len(fragments)} fragments -> {output_file}")
6878
subprocess.run(
69-
[str(bin_path), f"--bmf-out={out}"],
79+
[
80+
sys.executable,
81+
str(BENCHMARK_DIR / "merge_bmf.py"),
82+
str(output_file),
83+
*[str(p) for p in fragments],
84+
],
7085
check=True,
7186
)
72-
fragments.append(out)
73-
# Merge the 5 BMF fragments into the requested output file via
74-
# merge_bmf.py. Unions per-slug measure dicts and exits 1 on
75-
# (slug, measure) collisions.
76-
print(f"Merging {len(fragments)} fragments -> {output_file}")
77-
subprocess.run(
78-
[
79-
sys.executable,
80-
str(BENCHMARK_DIR / "merge_bmf.py"),
81-
str(output_file),
82-
*[str(p) for p in fragments],
83-
],
84-
check=True,
85-
)
86-
print(f"Combined results written to {output_file}")
87+
print(f"Combined results written to {output_file}")
88+
finally:
89+
# The merged file at output_file is the canonical artifact; the
90+
# per-binary fragments are intermediate. Drop them so RESULTS_DIR
91+
# doesn't accumulate one set per invocation. `finally` so a
92+
# bench-binary failure or a merge collision still cleans up
93+
# whatever fragments were produced.
94+
for f in fragments:
95+
f.unlink(missing_ok=True)
8796

8897

8998
def build(args):

0 commit comments

Comments
 (0)