Skip to content

Optimize DataStore versioning and reduce allocations#68

Open
haoruizhou wants to merge 1 commit intomainfrom
pecan-fixes
Open

Optimize DataStore versioning and reduce allocations#68
haoruizhou wants to merge 1 commit intomainfrom
pecan-fixes

Conversation

@haoruizhou
Copy link
Copy Markdown
Contributor

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.

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.
@haoruizhou
Copy link
Copy Markdown
Contributor Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines 617 to +618
this.notifyAll(msgID);
this._version++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +572 to +574
roundedData[key] = rounded === signal.sensorReading
? signal
: { ...signal, sensorReading: rounded };
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant