Skip to content

perf: avoid redundant internal state snapshots - #2025

Open
dougle03 wants to merge 4 commits into
zachowj:mainfrom
dougle03:perf/readonly-state-access
Open

perf: avoid redundant internal state snapshots#2025
dougle03 wants to merge 4 commits into
zachowj:mainfrom
dougle03:perf/readonly-state-access

Conversation

@dougle03

Copy link
Copy Markdown
Contributor

Summary

This is a focused, contract-preserving alternative to the broader performance work in #2011.

The Node-RED integration currently deep-clones the complete Home Assistant state map in getStates(). Several internal consumers only inspect that map while rendering templates, filtering entities, or checking conditions. This PR avoids those redundant clones without changing the public getter contract.

What changes

  • getStates() and getState() continue returning independent defensive copies.
  • A clearly named getStatesReadOnly() accessor is added for internal consumers that only read the state map.
  • Only audited read-only paths use the new accessor.
  • Template helper types accept a read-only top-level entity map.
  • Tests verify that public snapshots remain independent and that the read-only accessor exposes the current cache to internal consumers.

Why this differs from #2011

PR #2011 demonstrates a substantial CPU reduction, but it also exposes shared state through getStatesRef() and includes a broad set of changes. This PR keeps the optimisation narrowly scoped and makes the ownership rule explicit: public APIs remain safe snapshots; only audited internal readers may use the shared map.

It does not change the upstream home-assistant-js-websocket snapshot behaviour. That behaviour is intentional because listeners may retain old snapshots and compare them with new ones.

Testing

Automated

Run the normal project checks:

pnpm test
pnpm run lint

The focused tests verify:

  • Mutating getStates() does not alter the websocket cache.
  • getStatesReadOnly() returns the current map for explicitly read-only internal consumers.
  • TypeScript rejects passing the read-only map to APIs that require a mutable map unless the caller makes an explicit decision.

Manual regression testing

Against a high-update Home Assistant workload, compare the current release with this branch for:

  • CPU and allocation/GC pressure.
  • Current State, Get Entities, API/template rendering, Get History, Fire Event, Trigger State, Wait Until, Tag, and Zone nodes.
  • Entity additions, removals, state changes, and attribute changes.
  • Node-RED reconnects and independent Node-RED/Home Assistant restarts.
  • Public API safety: mutate data returned by getStates() and getState() and verify subsequent reads are unchanged.

The shared object returned by getStatesReadOnly() must never be mutated by a caller. A regression in entity counts, stale state, missing removals, reconnect handling, or public snapshot isolation should block adoption.

@dougle03

Copy link
Copy Markdown
Contributor Author

The first CI run failed in pnpm lint before tests could start. One indentation error was on a line changed by this PR and has now been corrected in commit 85615c20.

The remaining 11 Prettier errors are in files unchanged by PR #2025:

  • src/common/integration/BidirectionalEntityIntegration.ts
  • src/common/integration/UnidirectionalEntityIntegration.ts
  • src/editor/data.ts
  • src/nodes/config-server/EditorContext.ts
  • src/nodes/get-history/const.ts
  • src/nodes/number/NumberController.ts
  • src/nodes/select/SelectController.ts
  • src/nodes/text/TextController.ts

The branch has been updated so CI can confirm that the focused changes themselves are clean. I have not included unrelated repository-wide formatting changes in this PR. The test jobs were skipped after lint failed; they should run once lint passes or the baseline formatting issue is handled separately.

@dougle03

Copy link
Copy Markdown
Contributor Author

I have opened PR #2026 as a separate formatting-only prerequisite. It fixes only the existing Prettier errors currently blocking the repository lint gate; it contains no runtime or CPU changes. Once #2026 is merged, the full CI matrix for #2025 should be able to reach the actual tests.

@dougle03
dougle03 force-pushed the perf/readonly-state-access branch from 85615c2 to 5e574d7 Compare July 24, 2026 07:05
@dougle03

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current upstream main (bf577664) and force-pushed the updated commits. Local pnpm install --frozen-lockfile is currently blocked because the pnpm-lock.yaml already present on upstream main contains a duplicated YAML mapping key (reported at line 6489). PR #2025 does not modify pnpm-lock.yaml.

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