Skip to content

Commit 3cba505

Browse files
carlos-almcarlos-alm
authored
fix(bench): report partial native results when incremental rebuild fails (#741)
* fix(bench): report partial native results when incremental rebuild fails Wrap no-op and 1-file incremental rebuild phases in try/catch so that when buildGraph crashes (e.g. SQLITE_CORRUPT), the worker still emits JSON with null for the failed fields instead of crashing before output. Also protect the finally-block cleanup rebuild and update both report generators to render 'n/a' for null incremental values and skip null values in regression checks. Closes #716 * fix(bench): preserve partial incremental rows and guard trend() against null - Widen hasIncremental gate and row filter in update-benchmark-report.ts to account for oneFileRebuildMs independently of noopRebuildMs - Guard trend() against null current value in both report scripts * fix(bench): cast catch err to Error for type safety (#741)
1 parent 27a55c3 commit 3cba505

4 files changed

Lines changed: 67 additions & 28 deletions

File tree

‎scripts/benchmark.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,22 @@ const dbSizeBytes = fs.statSync(dbPath).size;
148148

149149
// ── Incremental build tiers ─────────────────────────────────────────
150150
console.error(` [${engine}] Benchmarking no-op rebuild...`);
151-
const noopTimings = [];
152-
for (let i = 0; i < INCREMENTAL_RUNS; i++) {
153-
const start = performance.now();
154-
await buildGraph(root, { engine, incremental: true });
155-
noopTimings.push(performance.now() - start);
151+
let noopRebuildMs = null;
152+
try {
153+
const noopTimings = [];
154+
for (let i = 0; i < INCREMENTAL_RUNS; i++) {
155+
const start = performance.now();
156+
await buildGraph(root, { engine, incremental: true });
157+
noopTimings.push(performance.now() - start);
158+
}
159+
noopRebuildMs = Math.round(median(noopTimings));
160+
} catch (err) {
161+
console.error(` [${engine}] No-op rebuild failed: ${(err as Error).message}`);
156162
}
157-
const noopRebuildMs = Math.round(median(noopTimings));
158163

159164
console.error(` [${engine}] Benchmarking 1-file rebuild...`);
160165
const original = fs.readFileSync(PROBE_FILE, 'utf8');
161-
let oneFileRebuildMs;
166+
let oneFileRebuildMs = null;
162167
let oneFilePhases = null;
163168
try {
164169
const oneFileRuns = [];
@@ -172,9 +177,15 @@ try {
172177
const medianRun = oneFileRuns[Math.floor(oneFileRuns.length / 2)];
173178
oneFileRebuildMs = Math.round(medianRun.ms);
174179
oneFilePhases = medianRun.phases;
180+
} catch (err) {
181+
console.error(` [${engine}] 1-file rebuild failed: ${(err as Error).message}`);
175182
} finally {
176183
fs.writeFileSync(PROBE_FILE, original);
177-
await buildGraph(root, { engine, incremental: true });
184+
try {
185+
await buildGraph(root, { engine, incremental: true });
186+
} catch {
187+
// Cleanup rebuild failed — probe file is already restored, move on
188+
}
178189
}
179190

180191
// ── Query benchmarks ────────────────────────────────────────────────

‎scripts/incremental-benchmark.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,22 @@ for (let i = 0; i < RUNS; i++) {
170170
const fullBuildMs = Math.round(median(fullTimings));
171171

172172
// No-op rebuild (nothing changed)
173-
const noopTimings = [];
174-
for (let i = 0; i < RUNS; i++) {
175-
const start = performance.now();
176-
await buildGraph(root, { engine, incremental: true });
177-
noopTimings.push(performance.now() - start);
173+
let noopRebuildMs = null;
174+
try {
175+
const noopTimings = [];
176+
for (let i = 0; i < RUNS; i++) {
177+
const start = performance.now();
178+
await buildGraph(root, { engine, incremental: true });
179+
noopTimings.push(performance.now() - start);
180+
}
181+
noopRebuildMs = Math.round(median(noopTimings));
182+
} catch (err) {
183+
console.error(` [${engine}] No-op rebuild failed: ${(err as Error).message}`);
178184
}
179-
const noopRebuildMs = Math.round(median(noopTimings));
180185

181186
// 1-file change rebuild
182187
const original = fs.readFileSync(PROBE_FILE, 'utf8');
183-
let oneFileRebuildMs;
188+
let oneFileRebuildMs = null;
184189
let oneFilePhases = null;
185190
try {
186191
const oneFileRuns = [];
@@ -194,12 +199,18 @@ try {
194199
const medianRun = oneFileRuns[Math.floor(oneFileRuns.length / 2)];
195200
oneFileRebuildMs = Math.round(medianRun.ms);
196201
oneFilePhases = medianRun.phases;
202+
} catch (err) {
203+
console.error(` [${engine}] 1-file rebuild failed: ${(err as Error).message}`);
197204
} finally {
198205
fs.writeFileSync(PROBE_FILE, original);
199-
await buildGraph(root, { engine, incremental: true });
206+
try {
207+
await buildGraph(root, { engine, incremental: true });
208+
} catch {
209+
// Cleanup rebuild failed — probe file is already restored, move on
210+
}
200211
}
201212

202-
console.error(` full=${fullBuildMs}ms noop=${noopRebuildMs}ms 1-file=${oneFileRebuildMs}ms`);
213+
console.error(` full=${fullBuildMs}ms noop=${noopRebuildMs ?? 'FAILED'}ms 1-file=${oneFileRebuildMs ?? 'FAILED'}ms`);
203214

204215
// Restore console.log for JSON output
205216
console.log = origLog;

‎scripts/update-benchmark-report.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function findPrevRelease(hist, fromIdx) {
6969

7070
// ── Helpers ──────────────────────────────────────────────────────────────
7171
function trend(current, previous, lowerIsBetter = true) {
72-
if (previous == null) return '';
72+
if (current == null || previous == null) return '';
7373
const pct = ((current - previous) / previous) * 100;
7474
if (Math.abs(pct) < 2) return ' ~';
7575
if (lowerIsBetter) {
@@ -207,7 +207,13 @@ md += `| Nodes | ${estNative ? Math.round(estNative.nodes * ESTIMATE_FILES).toLo
207207
md += `| Edges | ${estNative ? Math.round(estNative.edges * ESTIMATE_FILES).toLocaleString() : 'n/a'} | ${Math.round(estWasm.edges * ESTIMATE_FILES).toLocaleString()} |\n\n`;
208208

209209
// ── Incremental Rebuilds section ──────────────────────────────────────────
210-
const hasIncremental = history.some((h) => h.wasm?.noopRebuildMs != null || h.native?.noopRebuildMs != null);
210+
const hasIncremental = history.some(
211+
(h) =>
212+
h.wasm?.noopRebuildMs != null ||
213+
h.native?.noopRebuildMs != null ||
214+
h.wasm?.oneFileRebuildMs != null ||
215+
h.native?.oneFileRebuildMs != null,
216+
);
211217
if (hasIncremental) {
212218
md += '### Incremental Rebuilds\n\n';
213219
md += '| Version | Engine | No-op (ms) | 1-file (ms) |\n';
@@ -219,13 +225,15 @@ if (hasIncremental) {
219225

220226
for (const engineKey of ['native', 'wasm']) {
221227
const e = h[engineKey];
222-
if (!e || e.noopRebuildMs == null) continue;
228+
if (!e || (e.noopRebuildMs == null && e.oneFileRebuildMs == null)) continue;
223229
const p = prev?.[engineKey] || null;
224230

225231
const noopTrend = trend(e.noopRebuildMs, p?.noopRebuildMs);
232+
const noopCell = e.noopRebuildMs != null ? `${e.noopRebuildMs}${noopTrend}` : 'n/a';
226233
const oneFileTrend = trend(e.oneFileRebuildMs, p?.oneFileRebuildMs);
234+
const oneFileCell = e.oneFileRebuildMs != null ? `${e.oneFileRebuildMs}${oneFileTrend}` : 'n/a';
227235

228-
md += `| ${h.version} | ${engineKey} | ${e.noopRebuildMs}${noopTrend} | ${e.oneFileRebuildMs}${oneFileTrend} |\n`;
236+
md += `| ${h.version} | ${engineKey} | ${noopCell} | ${oneFileCell} |\n`;
229237
}
230238
}
231239
md += '\n';
@@ -334,6 +342,8 @@ if (fs.existsSync(readmePath)) {
334342
// Incremental rebuild rows (prefer native, fallback to WASM)
335343
if (pref.noopRebuildMs != null) {
336344
rows += `| No-op rebuild${prefLabel} | **${formatMs(pref.noopRebuildMs)}** |\n`;
345+
}
346+
if (pref.oneFileRebuildMs != null) {
337347
rows += `| 1-file rebuild${prefLabel} | **${formatMs(pref.oneFileRebuildMs)}** |\n`;
338348
}
339349

‎scripts/update-incremental-report.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function findPrevRelease(hist, fromIdx) {
6262

6363
// ── Helpers ──────────────────────────────────────────────────────────────
6464
function trend(current, previous, lowerIsBetter = true) {
65-
if (previous == null) return '';
65+
if (current == null || previous == null) return '';
6666
const pct = ((current - previous) / previous) * 100;
6767
if (Math.abs(pct) < 2) return ' ~';
6868
if (lowerIsBetter) {
@@ -90,11 +90,14 @@ function engineRow(h, prev, engineKey) {
9090
const natT = r.nativeBatchMs != null ? trend(r.nativeBatchMs, pr?.nativeBatchMs) : '';
9191
const jsT = trend(r.jsFallbackMs, pr?.jsFallbackMs);
9292

93+
const noopCell = e.noopRebuildMs != null ? `${formatMs(e.noopRebuildMs)}${noopT}` : 'n/a';
94+
const oneFileCell = e.oneFileRebuildMs != null ? `${formatMs(e.oneFileRebuildMs)}${oneT}` : 'n/a';
95+
9396
return (
9497
`| ${h.version} | ${engineKey} | ${h.files} ` +
9598
`| ${formatMs(e.fullBuildMs)}${fullT} ` +
96-
`| ${formatMs(e.noopRebuildMs)}${noopT} ` +
97-
`| ${formatMs(e.oneFileRebuildMs)}${oneT} ` +
99+
`| ${noopCell} ` +
100+
`| ${oneFileCell} ` +
98101
`| ${r.nativeBatchMs != null ? formatMs(r.nativeBatchMs) + natT : 'n/a'} ` +
99102
`| ${formatMs(r.jsFallbackMs)}${jsT} |`
100103
);
@@ -134,8 +137,8 @@ for (const engineKey of ['native', 'wasm']) {
134137
md += '| Metric | Value |\n';
135138
md += '|--------|------:|\n';
136139
md += `| Full build | ${formatMs(e.fullBuildMs)} |\n`;
137-
md += `| No-op rebuild | ${formatMs(e.noopRebuildMs)} |\n`;
138-
md += `| 1-file rebuild | ${formatMs(e.oneFileRebuildMs)} |\n\n`;
140+
md += `| No-op rebuild | ${e.noopRebuildMs != null ? formatMs(e.noopRebuildMs) : 'n/a'} |\n`;
141+
md += `| 1-file rebuild | ${e.oneFileRebuildMs != null ? formatMs(e.oneFileRebuildMs) : 'n/a'} |\n\n`;
139142
}
140143

141144
const r = latest.resolve;
@@ -182,8 +185,12 @@ if (prev) {
182185
if (!e || !p) continue;
183186
const tag = `[${engineKey}]`;
184187
checkRegression(`${tag} Full build`, e.fullBuildMs, p.fullBuildMs);
185-
checkRegression(`${tag} No-op rebuild`, e.noopRebuildMs, p.noopRebuildMs);
186-
checkRegression(`${tag} 1-file rebuild`, e.oneFileRebuildMs, p.oneFileRebuildMs);
188+
if (e.noopRebuildMs != null && p.noopRebuildMs != null) {
189+
checkRegression(`${tag} No-op rebuild`, e.noopRebuildMs, p.noopRebuildMs);
190+
}
191+
if (e.oneFileRebuildMs != null && p.oneFileRebuildMs != null) {
192+
checkRegression(`${tag} 1-file rebuild`, e.oneFileRebuildMs, p.oneFileRebuildMs);
193+
}
187194
}
188195
const re = latest.resolve;
189196
const rp = prev.resolve;

0 commit comments

Comments
 (0)