mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-23 15:14:01 +00:00
Fixes #1258 — Perf dashboard (/#/perf) was slow because of three frontend issues; backend APIs were never the problem. ## Findings 1. **`/api/health` fetched sequentially after `Promise.all`** in `refresh()` — added a full RTT (~50-200ms) on every 5s tick on top of the parallel batch. 2. **Endpoints table not actually sorted** despite the heading "sorted by total time". JSON shape is `map[string]EndpointStatsResp` (no defined order); frontend rendered map iteration order. Visible correctness bug surfaced during investigation. 3. **`setInterval(refresh, 5000)` kept firing while tab was hidden**, rebuilding the entire ~10-section `innerHTML` (cards + 3 tables) in the background. On tab return the user saw a backlog thrash + felt the page was "slow to render". ## Fix (`public/perf.js`) - Move `/api/health` into the same `Promise.all` as the other 4 endpoints — saves one RTT per refresh. - Sort `Object.entries(server.endpoints)` by `count * avgMs` DESC client-side. - Add `document.hidden` guard in the interval tick + `visibilitychange` listener that refreshes once on return; `destroy()` removes the listener. ## Tests `test-perf-render-1258.js` (new): - All 5 initial fetches issued in parallel (including `/api/health`) - Refresh suppressed while `document.hidden` - Endpoints table sorted by total time DESC, regardless of input map order RED commit first (`6b54f9e8`, 0/3 pass) → GREEN commit (`be81303b`, 3/3 pass). Existing `test-perf-go-runtime.js` (13/13) and `test-perf-disk-io-1120.js` (15/15) still green. ## Investigation exemption No Playwright timing test — sandbox can't run a real browser. Static analysis + render-shape unit tests cover the three identified bottlenecks. Documented per AGENTS "investigation surfaces" exemption. ## Measurement Before: refresh = parallel batch (~max(server-side)) + sequential `/api/health` (~50ms) + full innerHTML rebuild every 5s including hidden tabs. After: refresh = single parallel batch, runs only while visible. Expected improvement on tab-return ≈ -1 RTT per refresh + zero background work. --------- Co-authored-by: corescope-bot <bot@corescope.local>
This commit is contained in:
1 parent
1efe93d7f6
commit
094a96bd6c
2 files changed
+173
-8
No files matched your search
+33
-8
@@ -13,17 +13,17 @@
|
||||
const el = document.getElementById('perfContent');
|
||||
if (!el) return;
|
||||
try {
|
||||
const [server, client, ioStats, sqliteStats, writeSources] = await Promise.all([
|
||||
// #1258: /api/health was awaited AFTER Promise.all, adding a full RTT
|
||||
// (~50-200ms) on every 5s refresh. Issue it in parallel with the rest.
|
||||
const [server, client, ioStats, sqliteStats, writeSources, health] = await Promise.all([
|
||||
fetch('/api/perf').then(r => r.json()),
|
||||
Promise.resolve(window.apiPerf ? window.apiPerf() : null),
|
||||
fetch('/api/perf/io').then(r => r.json()).catch(() => null),
|
||||
fetch('/api/perf/sqlite').then(r => r.json()).catch(() => null),
|
||||
fetch('/api/perf/write-sources').then(r => r.json()).catch(() => null)
|
||||
fetch('/api/perf/write-sources').then(r => r.json()).catch(() => null),
|
||||
fetch('/api/health').then(r => r.json()).catch(() => null)
|
||||
]);
|
||||
|
||||
// Also fetch health telemetry
|
||||
const health = await fetch('/api/health').then(r => r.json()).catch(() => null);
|
||||
|
||||
let html = '';
|
||||
|
||||
// Server overview
|
||||
@@ -230,8 +230,15 @@
|
||||
html += `</div>`;
|
||||
}
|
||||
|
||||
// Server endpoints table
|
||||
const eps = Object.entries(server.endpoints);
|
||||
// Server endpoints table — sort by total time (count * avg) DESC.
|
||||
// #1258: header claimed "sorted by total time" but JSON map order is
|
||||
// undefined and the frontend was not sorting. Slow endpoints could
|
||||
// appear anywhere in the table, defeating the section's whole purpose.
|
||||
const eps = Object.entries(server.endpoints).sort((a, b) => {
|
||||
const ta = (a[1].count || 0) * (a[1].avgMs || 0);
|
||||
const tb = (b[1].count || 0) * (b[1].avgMs || 0);
|
||||
return tb - ta;
|
||||
});
|
||||
if (eps.length) {
|
||||
html += '<h3>Server Endpoints (sorted by total time)</h3>';
|
||||
html += '<div style="overflow-x:auto"><table class="perf-table"><thead><tr><th scope="col">Endpoint</th><th scope="col">Count</th><th scope="col">Avg</th><th scope="col">P50</th><th scope="col">P95</th><th scope="col">Max</th><th scope="col">Total</th></tr></thead><tbody>';
|
||||
@@ -281,10 +288,28 @@
|
||||
registerPage('perf', {
|
||||
init(app) {
|
||||
render(app);
|
||||
interval = setInterval(refresh, 5000);
|
||||
// #1258: don't burn CPU/network rebuilding the page (and its many cards
|
||||
// + 3 large tables) every 5s while the tab is hidden. Pause polling on
|
||||
// visibilitychange and resume on focus. Reduces background fetch traffic
|
||||
// to zero and prevents a returning user from seeing a 100+ms thrash as
|
||||
// a backlog of refreshes flush.
|
||||
const tick = () => {
|
||||
if (document.hidden) return;
|
||||
refresh();
|
||||
};
|
||||
interval = setInterval(tick, 5000);
|
||||
const onVis = () => {
|
||||
if (!document.hidden) refresh();
|
||||
};
|
||||
document.addEventListener('visibilitychange', onVis);
|
||||
this._onVis = onVis;
|
||||
},
|
||||
destroy() {
|
||||
if (interval) { clearInterval(interval); interval = null; }
|
||||
if (this._onVis) {
|
||||
document.removeEventListener('visibilitychange', this._onVis);
|
||||
this._onVis = null;
|
||||
}
|
||||
}
|
||||
});
|
||||
})();
|
||||
@@ -0,0 +1,140 @@
|
||||
/* Tests for perf.js render performance (#1258).
|
||||
*
|
||||
* Failure modes we gate against:
|
||||
* 1) /api/health awaited sequentially AFTER Promise.all → extra RTT
|
||||
* 2) setInterval keeps polling even when document is hidden → wasted work
|
||||
* 3) Endpoints table claims "sorted by total time" but renders in map order
|
||||
*/
|
||||
'use strict';
|
||||
const vm = require('vm');
|
||||
const fs = require('fs');
|
||||
const assert = require('assert');
|
||||
|
||||
let passed = 0, failed = 0;
|
||||
function test(name, fn) {
|
||||
const run = (r) => { if (r && typeof r.then === 'function') return r.then(() => { passed++; console.log(` ✅ ${name}`); }, e => { failed++; console.log(` ❌ ${name}: ${e.message}`); }); passed++; console.log(` ✅ ${name}`); };
|
||||
try { const r = fn(); if (r && typeof r.then === 'function') return r.then(() => { passed++; console.log(` ✅ ${name}`); }, e => { failed++; console.log(` ❌ ${name}: ${e.message}`); }); else { passed++; console.log(` ✅ ${name}`); } }
|
||||
catch (e) { failed++; console.log(` ❌ ${name}: ${e.message}`); }
|
||||
}
|
||||
|
||||
function makeSandbox(opts = {}) {
|
||||
let capturedHtml = '';
|
||||
const pages = {};
|
||||
let visState = opts.hidden ? 'hidden' : 'visible';
|
||||
const visListeners = [];
|
||||
const ctx = {
|
||||
window: { addEventListener: () => {}, apiPerf: null },
|
||||
document: {
|
||||
getElementById: (id) => {
|
||||
if (id === 'perfContent') return { set innerHTML(v) { capturedHtml = v; } };
|
||||
if (id === 'perfReset' || id === 'perfRefresh') return { addEventListener: () => {} };
|
||||
return null;
|
||||
},
|
||||
addEventListener: (ev, fn) => { if (ev === 'visibilitychange') visListeners.push(fn); },
|
||||
removeEventListener: () => {},
|
||||
get visibilityState() { return visState; },
|
||||
get hidden() { return visState === 'hidden'; },
|
||||
},
|
||||
console,
|
||||
Date, Math, Array, Object, String, Number, JSON, RegExp, Error, TypeError,
|
||||
parseInt, parseFloat, isNaN, isFinite,
|
||||
setTimeout: (fn, ms) => setTimeout(fn, ms), clearTimeout,
|
||||
setInterval: (fn, ms) => { return setInterval(fn, ms); }, clearInterval,
|
||||
performance: { now: () => Date.now() },
|
||||
Map, Set, Promise,
|
||||
registerPage: (name, handler) => { pages[name] = handler; },
|
||||
_apiCache: { size: 0 },
|
||||
fetch: () => Promise.resolve({ json: () => Promise.resolve({}) }),
|
||||
};
|
||||
ctx.window.document = ctx.document;
|
||||
ctx.globalThis = ctx;
|
||||
return { ctx, pages, getHtml: () => capturedHtml,
|
||||
setVisibility(v) { visState = v; visListeners.forEach(fn => fn()); } };
|
||||
}
|
||||
|
||||
function loadPerf() {
|
||||
const sb = makeSandbox();
|
||||
const code = fs.readFileSync('public/perf.js', 'utf8');
|
||||
vm.runInNewContext(code, sb.ctx);
|
||||
return sb;
|
||||
}
|
||||
|
||||
// ---------- 1) Health fetched in parallel ----------
|
||||
test('all initial fetches (including /api/health) issued in parallel', async () => {
|
||||
const sb = loadPerf();
|
||||
const order = [];
|
||||
let resolveAll;
|
||||
const gate = new Promise(r => { resolveAll = r; });
|
||||
sb.ctx.fetch = (url) => {
|
||||
order.push(url);
|
||||
// Don't resolve until all 5 calls have been issued — proves they're parallel
|
||||
return gate.then(() => ({ json: () => Promise.resolve({}) }));
|
||||
};
|
||||
const p = sb.pages.perf.init({ set innerHTML(v) {} });
|
||||
// Microtask flush
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
// Before any fetch resolves, all 5 URLs must have been started
|
||||
assert.ok(order.includes('/api/health'),
|
||||
`expected /api/health to be issued in parallel with the others, got: ${order.join(', ')}`);
|
||||
resolveAll();
|
||||
await p;
|
||||
});
|
||||
|
||||
// ---------- 2) setInterval pauses when tab hidden ----------
|
||||
test('refresh interval does not fire when document is hidden', async () => {
|
||||
const sb = loadPerf();
|
||||
let fetchCount = 0;
|
||||
sb.ctx.fetch = (url) => {
|
||||
fetchCount++;
|
||||
return Promise.resolve({ json: () => Promise.resolve({}) });
|
||||
};
|
||||
// Replace setInterval with fast firing
|
||||
let timerFn = null;
|
||||
sb.ctx.setInterval = (fn, ms) => { timerFn = fn; return 1; };
|
||||
sb.ctx.clearInterval = () => { timerFn = null; };
|
||||
|
||||
await sb.pages.perf.init({ set innerHTML(v) {} });
|
||||
await new Promise(r => setTimeout(r, 30));
|
||||
const baseline = fetchCount;
|
||||
// Hide the tab, then fire the interval — should NOT issue fresh fetches
|
||||
sb.setVisibility('hidden');
|
||||
if (timerFn) timerFn();
|
||||
await new Promise(r => setTimeout(r, 30));
|
||||
assert.strictEqual(fetchCount, baseline,
|
||||
`refresh should be suppressed while hidden; baseline=${baseline} after=${fetchCount}`);
|
||||
});
|
||||
|
||||
// ---------- 3) Endpoints table actually sorted by total time ----------
|
||||
test('endpoints table is sorted by total time descending', async () => {
|
||||
const sb = loadPerf();
|
||||
// Map insertion order is preserved in JS object literals — put SLOW endpoint
|
||||
// LAST to ensure the renderer is actively sorting, not relying on input order.
|
||||
const perfData = {
|
||||
totalRequests: 100, avgMs: 5, uptime: 3600, slowQueries: [],
|
||||
endpoints: {
|
||||
'/api/fast': { count: 1, avgMs: 1, p50Ms: 1, p95Ms: 1, maxMs: 1 },
|
||||
'/api/mid': { count: 10, avgMs: 10, p50Ms: 10, p95Ms: 10, maxMs: 10 },
|
||||
'/api/SLOW': { count: 100, avgMs: 100, p50Ms: 100, p95Ms: 100, maxMs: 100 },
|
||||
},
|
||||
};
|
||||
sb.ctx.fetch = (url) => {
|
||||
if (url === '/api/perf') return Promise.resolve({ json: () => Promise.resolve(perfData) });
|
||||
return Promise.resolve({ json: () => Promise.resolve(null) });
|
||||
};
|
||||
await sb.pages.perf.init({ set innerHTML(v) {} });
|
||||
await new Promise(r => setTimeout(r, 30));
|
||||
const html = sb.getHtml();
|
||||
const iSlow = html.indexOf('/api/SLOW');
|
||||
const iMid = html.indexOf('/api/mid');
|
||||
const iFast = html.indexOf('/api/fast');
|
||||
assert.ok(iSlow > -1 && iMid > -1 && iFast > -1, 'all three endpoints must render');
|
||||
assert.ok(iSlow < iMid && iMid < iFast,
|
||||
`expected SLOW < mid < fast in DOM order, got SLOW=${iSlow} mid=${iMid} fast=${iFast}`);
|
||||
});
|
||||
|
||||
setTimeout(() => {
|
||||
console.log(`\n${passed} passed, ${failed} failed\n`);
|
||||
process.exit(failed ? 1 : 0);
|
||||
}, 500);
|
||||
Reference in new issue
Block a user