From 07bd4107cca5825f036bae3bb2a2c0605aa4d95e Mon Sep 17 00:00:00 2001 From: Haorui Zhou Date: Sat, 9 May 2026 10:58:09 -0400 Subject: [PATCH] Optimize DataStore versioning and reduce allocations Add a monotonic _version counter to DataStore (getVersion) and increment it on all mutating operations so subscribers can skip redundant work. Reduce allocations when rounding sensor readings by only cloning signal objects if rounding changes the value (preserving identity for common cases). Update useAllLatestMessages to snapshot the version and avoid unnecessary Map rebuilds, and batch cold-store state into a single object to cut re-renders. Simplify Trace memo usage that was a no-op. Add tests for version tick and ingestMessage data handling (rounding, immutability, defaults). Files changed: DataStore.ts, useDataStore.ts, Trace.tsx, and DataStore.test.ts. --- pecan/src/lib/DataStore.test.ts | 97 +++++++++++++++++++++++++++++++++ pecan/src/lib/DataStore.ts | 64 ++++++++++++++++------ pecan/src/lib/useDataStore.ts | 54 +++++++++++------- pecan/src/pages/Trace.tsx | 6 +- 4 files changed, 180 insertions(+), 41 deletions(-) diff --git a/pecan/src/lib/DataStore.test.ts b/pecan/src/lib/DataStore.test.ts index 116c7a1..d3accc6 100644 --- a/pecan/src/lib/DataStore.test.ts +++ b/pecan/src/lib/DataStore.test.ts @@ -208,3 +208,100 @@ describe('DataStore', () => { expect(dataStore.getLatest('M2')?.rawData).toBe('11'); }); }); + +// ── P0-2: version-counter ───────────────────────────────────────────────────── +// DataStore exposes a version tick that increments on every write so subscribers +// (specifically useAllLatestMessages) can skip redundant Map rebuilds. +describe('DataStore version tick (P0-2)', () => { + beforeEach(() => { + dataStore.clear(); + dataStore.setRetentionWindow(30000); + vi.useFakeTimers(); + }); + afterEach(() => { vi.useRealTimers(); }); + + it('increments version on every ingestMessage call', () => { + const base = Date.now(); + const v0 = dataStore.getVersion(); + dataStore.ingestMessage({ msgID: 'V1', messageName: 'V1', data: {}, rawData: '01', timestamp: base }); + const v1 = dataStore.getVersion(); + expect(v1).toBeGreaterThan(v0); + + dataStore.ingestMessage({ msgID: 'V1', messageName: 'V1', data: {}, rawData: '02', timestamp: base + 1 }); + expect(dataStore.getVersion()).toBeGreaterThan(v1); + }); + + it('increments version on ingestMessagesBatch', () => { + const base = Date.now(); + const v0 = dataStore.getVersion(); + dataStore.ingestMessagesBatch([ + { msgID: 'B1', messageName: 'B1', data: {}, rawData: '01', timestamp: base }, + { msgID: 'B2', messageName: 'B2', data: {}, rawData: '02', timestamp: base }, + ]); + expect(dataStore.getVersion()).toBeGreaterThan(v0); + }); + + it('increments version on clear and clearMessage', () => { + const base = Date.now(); + dataStore.ingestMessage({ msgID: 'C1', messageName: 'C1', data: {}, rawData: '01', timestamp: base }); + const v0 = dataStore.getVersion(); + + dataStore.clearMessage('C1'); + expect(dataStore.getVersion()).toBeGreaterThan(v0); + + dataStore.ingestMessage({ msgID: 'C2', messageName: 'C2', data: {}, rawData: '02', timestamp: base }); + const v2 = dataStore.getVersion(); + dataStore.clear(); + expect(dataStore.getVersion()).toBeGreaterThan(v2); + }); +}); + +// ── P0-3: ingestMessage allocation correctness ───────────────────────────────── +describe('DataStore ingestMessage data handling (P0-3)', () => { + beforeEach(() => { + dataStore.clear(); + dataStore.setRetentionWindow(30000); + vi.useFakeTimers(); + }); + afterEach(() => { vi.useRealTimers(); }); + + it('rounds fractional sensor readings to 3 decimal places', () => { + const base = Date.now(); + dataStore.ingestMessage({ + msgID: 'R1', + messageName: 'R1', + data: { rpm: { sensorReading: 1234.56789, unit: 'rpm' } }, + rawData: 'BB', + timestamp: base, + }); + expect(dataStore.getLatest('R1')!.data.rpm.sensorReading).toBe(1234.568); + }); + + it('does not mutate the original data object passed to ingestMessage', () => { + const base = Date.now(); + const originalData = { temp: { sensorReading: 99.999999, unit: 'C' } }; + dataStore.ingestMessage({ + msgID: 'R2', + messageName: 'R2', + data: originalData, + rawData: 'CC', + timestamp: base, + }); + // Store must never alias the caller's object. + expect(dataStore.getLatest('R2')!.data).not.toBe(originalData); + // And must not have mutated the original. + expect(originalData.temp.sensorReading).toBe(99.999999); + }); + + it('handles empty data gracefully', () => { + const base = Date.now(); + dataStore.ingestMessage({ msgID: 'E1', messageName: 'E1', data: {}, rawData: '', timestamp: base }); + expect(dataStore.getLatest('E1')!.data).toEqual({}); + }); + + it('defaults direction to rx when not provided', () => { + const base = Date.now(); + dataStore.ingestMessage({ msgID: 'D1', messageName: 'D1', data: {}, rawData: '00', timestamp: base }); + expect(dataStore.getLatest('D1')!.direction).toBe('rx'); + }); +}); diff --git a/pecan/src/lib/DataStore.ts b/pecan/src/lib/DataStore.ts index c08b9d1..1c4897b 100644 --- a/pecan/src/lib/DataStore.ts +++ b/pecan/src/lib/DataStore.ts @@ -159,6 +159,9 @@ class DataStore { private recoveredFromSnapshot = false; private isRestoringSnapshot = false; + /** P0-2: monotonic write counter — subscribers can use this to skip redundant Map rebuilds */ + private _version = 0; + // ── Cold store integration ────────────────────────────────────────────── /** Decoded warm cache: cold frames re-decoded for the current scrub window. */ @@ -556,17 +559,23 @@ class DataStore { const msgID = message.msgID; - // Round sensor readings to 3 decimal places for cleaner display - const roundedData = { ...message.data }; - Object.keys(roundedData).forEach((key) => { - const signal = roundedData[key]; + // P0-3: only allocate new signal objects when rounding actually changes the value. + // This avoids a per-frame { ...message.data } spread and per-signal spread for + // signals that are already at 3 decimal places. + const roundedData: typeof message.data = {}; + for (const key of Object.keys(message.data)) { + const signal = message.data[key]; if (signal && typeof signal.sensorReading === 'number') { - roundedData[key] = { - ...signal, - sensorReading: Math.round(signal.sensorReading * 1000) / 1000, - }; + const rounded = Math.round(signal.sensorReading * 1000) / 1000; + // Only allocate a new signal object if rounding changed the value. + // This preserves object identity for the common case (already 3 dp). + roundedData[key] = rounded === signal.sensorReading + ? signal + : { ...signal, sensorReading: rounded }; + } else { + roundedData[key] = signal; } - }); + } // Create the sample const sample: TelemetrySample = { @@ -606,6 +615,7 @@ class DataStore { // Notify all subscribers this.notifyAll(msgID); + this._version++; } /** @@ -645,16 +655,19 @@ class DataStore { const buffers = this.getSourceBuffers(source); - const roundedData = { ...message.data }; - Object.keys(roundedData).forEach((key) => { - const signal = roundedData[key]; - if (signal && typeof signal.sensorReading === "number") { - roundedData[key] = { - ...signal, - sensorReading: Math.round(signal.sensorReading * 1000) / 1000, - }; + // P0-3: same optimisation as ingestMessage — skip allocations when rounding is a no-op. + const roundedData: typeof message.data = {}; + for (const key of Object.keys(message.data)) { + const signal = message.data[key]; + if (signal && typeof signal.sensorReading === 'number') { + const rounded = Math.round(signal.sensorReading * 1000) / 1000; + roundedData[key] = rounded === signal.sensorReading + ? signal + : { ...signal, sensorReading: rounded }; + } else { + roundedData[key] = signal; } - }); + } const sample: TelemetrySample = { timestamp, @@ -689,6 +702,7 @@ class DataStore { this.scheduleSnapshotSave(); this.notifyTrace(); this.notifyAll(); + this._version++; } /** @@ -981,6 +995,7 @@ class DataStore { this.scheduleSnapshotSave(); this.notifyAll(); this.notifyTrace(); + this._version++; } // ── Cold state / warm cache API ─────────────────────────────────────────── @@ -1143,6 +1158,7 @@ class DataStore { this.getSourceBuffers(source).trace = []; this.scheduleSnapshotSave(); this.notifyTrace(); + this._version++; } private pruneTraceBuffer(referenceTimeMs: number, source: TelemetrySource = this.activeSource): void { @@ -1224,6 +1240,7 @@ class DataStore { this.getSourceBuffers(source).byMsgId.delete(msgID); this.scheduleSnapshotSave(); this.notifyAll(msgID); + this._version++; } /** @@ -1236,6 +1253,7 @@ class DataStore { } this.retentionWindowMs = windowMs; + this._version++; // Prune all messages with new window for (const source of ["live", "replay"] as TelemetrySource[]) { @@ -1257,6 +1275,16 @@ class DataStore { this.notifyTrace(); } + /** + * Returns the current monotonic write version. + * Incremented on every mutating operation (ingest, clear, etc.). + * Subscribers can snapshot this value and skip expensive recomputations + * when the version hasn't changed since their last render. + */ + public getVersion(): number { + return this._version; + } + /** * Get current retention window * @returns Retention window in milliseconds diff --git a/pecan/src/lib/useDataStore.ts b/pecan/src/lib/useDataStore.ts index 5045d63..05f55c9 100644 --- a/pecan/src/lib/useDataStore.ts +++ b/pecan/src/lib/useDataStore.ts @@ -101,23 +101,29 @@ export function useSignal(msgID: string, signalName: string): { } /** - * Hook to get all latest messages - * Updates whenever any message is updated - * + * Hook to get all latest messages. + * Uses the DataStore version counter to skip redundant Map rebuilds — the Map + * is only reconstructed when the version has actually changed since last render. + * * @returns Map of msgID to latest telemetry sample */ export function useAllLatestMessages(source?: TelemetrySource): Map { - const [allLatest, setAllLatest] = useState>(() => + const [allLatest, setAllLatest] = useState>(() => dataStore.getAllLatest(source) ); useEffect(() => { - // Initial value + // P0-2: snapshot the version at subscribe time; only rebuild the Map + // when the version has actually changed (i.e. data was written). + let lastVersion = dataStore.getVersion(); setAllLatest(dataStore.getAllLatest(source)); - // Subscribe to all updates const unsubscribe = dataStore.subscribe(() => { - setAllLatest(dataStore.getAllLatest(source)); + const currentVersion = dataStore.getVersion(); + if (currentVersion !== lastVersion) { + lastVersion = currentVersion; + setAllLatest(dataStore.getAllLatest(source)); + } }); return unsubscribe; @@ -218,29 +224,35 @@ export function useColdStoreState(): { coldDurationMs: number; coldNearingLimit: boolean; } { - const [isLoading, setIsLoading] = useState(() => dataStore.isColdCacheLoading()); - const [coldWarning, setColdWarning] = useState(null); - const [coldSizeBytes, setColdSizeBytes] = useState(() => dataStore.getColdStoreSizeBytes()); - const [coldDurationMs, setColdDurationMs] = useState(() => dataStore.getColdExtent()?.endMs - ? (dataStore.getColdExtent()!.endMs - dataStore.getColdExtent()!.startMs) : 0); - const [coldNearingLimit, setColdNearingLimit] = useState(() => dataStore.isColdNearingLimit()); + const [coldStoreState, setColdStoreState] = useState(() => ({ + isLoading: dataStore.isColdCacheLoading(), + coldWarning: null as string | null, + coldSizeBytes: dataStore.getColdStoreSizeBytes(), + coldDurationMs: (() => { + const e = dataStore.getColdExtent(); + return e ? e.endMs - e.startMs : 0; + })(), + coldNearingLimit: dataStore.isColdNearingLimit(), + })); useEffect(() => { + // P0-2 companion: batch all state updates into a single setState to avoid + // N separate re-renders (one per field) whenever cold state changes. const unsubscribe = dataStore.subscribeColdState(() => { - setIsLoading(dataStore.isColdCacheLoading()); - const warning = dataStore.consumeColdWarning(); - if (warning) setColdWarning(warning); - - setColdSizeBytes(dataStore.getColdStoreSizeBytes()); const extent = dataStore.getColdExtent(); - setColdDurationMs(extent ? extent.endMs - extent.startMs : 0); - setColdNearingLimit(dataStore.isColdNearingLimit()); + setColdStoreState({ + isLoading: dataStore.isColdCacheLoading(), + coldWarning: warning ?? null, + coldSizeBytes: dataStore.getColdStoreSizeBytes(), + coldDurationMs: extent ? extent.endMs - extent.startMs : 0, + coldNearingLimit: dataStore.isColdNearingLimit(), + }); }); return unsubscribe; }, []); - return { isLoading, coldWarning, coldSizeBytes, coldDurationMs, coldNearingLimit }; + return coldStoreState; } /** diff --git a/pecan/src/pages/Trace.tsx b/pecan/src/pages/Trace.tsx index 7937284..5c156d6 100644 --- a/pecan/src/pages/Trace.tsx +++ b/pecan/src/pages/Trace.tsx @@ -496,8 +496,10 @@ function Trace() { }); }, [frames, filter, paused, selectedTimeMs]); - const enriched = useMemo(() => buildEnriched(filteredFrames), [filteredFrames]); - const fixed = useMemo(() => enriched, [enriched]); + // buildEnriched is cheap; no need for a chain of memos. + // fixed was a no-op: useMemo(() => enriched, [enriched]) always === enriched. + const enriched = buildEnriched(filteredFrames); + const fixed = enriched; const totalFrames = frames.length;