Optimize DataStore versioning and reduce allocations#68
Optimize DataStore versioning and reduce allocations#68haoruizhou wants to merge 1 commit intomainfrom
Conversation
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07bd4107cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.notifyAll(msgID); | ||
| this._version++; |
There was a problem hiding this comment.
Move the version bump before notifying subscribers
When a live frame is ingested after useAllLatestMessages has subscribed, the subscriber runs during notifyAll while getVersion() still returns the old value, so the hook's currentVersion !== lastVersion guard skips rebuilding the latest-message Map. A dashboard that is mounted before the first/only CAN frame will not show it until a later write, and live data is always one notification behind; the same ordering appears in batch/clear paths, so the version needs to be incremented before notifying.
Useful? React with 👍 / 👎.
| roundedData[key] = rounded === signal.sensorReading | ||
| ? signal | ||
| : { ...signal, sensorReading: rounded }; |
There was a problem hiding this comment.
Copy signals even when rounding is a no-op
For any signal whose reading is already rounded, the stored sample now keeps the caller's signal object by reference. If the decoded message object is reused or mutated after ingestMessage, historical data in DataStore changes as well; the previous implementation always spread each signal and the new immutability test only covers the changed-rounding case, so no-op values still alias the caller.
Useful? React with 👍 / 👎.
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.