Skip to content

[codex] Improve high-event hot paths#1389

Merged
helto4real merged 5 commits into
mainfrom
codex/latest-origin-main
Jul 10, 2026
Merged

[codex] Improve high-event hot paths#1389
helto4real merged 5 commits into
mainfrom
codex/latest-origin-main

Conversation

@helto4real

@helto4real helto4real commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Breaking change

None. This keeps public APIs unchanged and preserves raw OnHassMessage publication behavior.

Proposed change

This PR implements the measured high-event performance improvements from the performance review plan.

The command/result hot path now uses an ID-indexed pending-result table instead of creating one filtered Rx subscription per command. HomeAssistantConnection registers a TaskCompletionSource<HassMessage> for each command id, completes matching result messages directly in the receive loop, and still publishes every raw HassMessage to OnHassMessage for ping, trigger, and diagnostic consumers.

Review follow-up fixed the pending-result lifecycle: pending entries are removed when a result arrives, the connection is disposed, sending fails, the caller cancels while waiting, or the result wait times out. Tests cover timeout and cancellation cleanup.

The websocket receive path reads a complete websocket message into a byte buffer, detects whether the payload is a JSON object or coalesced array, and deserializes directly to T or T[]. This removes the prior JsonElement intermediate and second typed deserialization on the ingest path while preserving chunked message and remote-close behavior.

The benchmark harness keeps the owning JsonDocument alive for the state-change benchmark lifetime, fixing the previous invalid JsonElement baseline failure.

Review follow-up also corrected ResultDispatchBenchmarks so the implemented path uses ConcurrentDictionary and awaits all completion tasks, matching the Rx baseline's completed-work shape. The corrected local in-process result-dispatch run showed the expected fixed overhead for one pending command and large wins as pending command count grows:

Pending commands Baseline Rx dispatch Concurrent dictionary dispatch Result
1 250.2 ns, 880 B 485.2 ns, 1,800 B slower for a single waiter
10 2,175.3 ns, 6,424 B 929.4 ns, 3,312 B 57% faster, 48% lower allocation
100 30,939.1 ns, 133,144 B 7,566.0 ns, 24,880 B 76% faster, 81% lower allocation
1000 1,819.0 us, 8,528,351 B 80.2 us, 262,848 B 96% faster, 97% lower allocation

Detailed local report: docs/performance/high-event-review.md.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • The code compiles without warnings (code quality check)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration are added/changed:

No user-facing functionality or configuration was added or changed.

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.47059% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81%. Comparing base (b913b8f) to head (73a4af3).

Files with missing lines Patch % Lines
...mon.HassClient/Internal/HomeAssistantConnection.cs 95% 1 Missing and 1 partial ⚠️
...sClient/Internal/Net/WebSocketTransportPipeline.cs 96% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@         Coverage Diff          @@
##           main   #1389   +/-   ##
====================================
- Coverage    81%     81%   -1%     
====================================
  Files       202     202           
  Lines      3941    3973   +32     
  Branches    444     453    +9     
====================================
+ Hits       3218    3241   +23     
- Misses      531     541   +10     
+ Partials    192     191    -1     
Flag Coverage Δ
unittests 81% <96%> (-1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@helto4real helto4real marked this pull request as ready for review July 1, 2026 15:28
@helto4real helto4real force-pushed the codex/latest-origin-main branch from 2d80d9a to e7218ba Compare July 5, 2026 13:42

@helto4real helto4real left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code review summary: the optimization direction looks sound and CI is green, but the pending-result lifecycle should be fixed before merging, and the benchmark should compare equivalent production paths before the precise performance claims are retained.

Comment thread src/Client/NetDaemon.HassClient/Internal/HomeAssistantConnection.cs
Comment thread src/Client/NetDaemon.HassClient/Internal/HomeAssistantConnection.cs Outdated
Comment thread benchmarks/NetDaemon.PerformanceBenchmarks/Benchmarks/ResultDispatchBenchmarks.cs Outdated
@helto4real helto4real merged commit 1d8c77e into main Jul 10, 2026
7 checks passed
@helto4real helto4real deleted the codex/latest-origin-main branch July 10, 2026 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants