perf(device): an idle connected device no longer burns CPU every second

[tylerkron]

What was wrong

A DAQiFi device that is connected but doing absolutely nothing was not free. Every connected device woke a background thread ten times a second forever, and every second it also ran a whole-system scan of the machine's serial ports just to confirm its own port was still there. On a long-running consumer — the desktop app or the MCP server, which hold devices open for hours — that is continuous background CPU and garbage for no work, and it got worse with every extra device connected.

Measured on the bench with one device connected and completely idle, that came to ~250 ms of CPU and 68 KiB of garbage per 30 seconds, per device.

How it was fixed

Two independent changes, neither of which alters what the library does:

• The producer's wait no longer has a timeout. It was polling on a 100 ms timeout, but that timeout was never load-bearing: everything the loop needs to notice (a queued message, a stop request) already sets the event before the loop looks, and the event is sticky, so nothing can be missed. An idle producer now wakes exactly zero times.
• The port-presence probe asks the cheap question first. It was calling SerialPort.GetPortNames() and only falling back to "does the device node still exist". Those two are OR-ed, so the order cannot change the answer — and on macOS/Linux, where a port name is a path, the filesystem check answers outright and the system-wide enumeration never runs. When it does run (Windows), it comes from one snapshot shared by every polling transport instead of one scan per device.

The one thing worth pushing back on is the snapshot's staleness. A cached "this port is present" can be up to a second old, so worst-case unplug detection moves from roughly three seconds to roughly four. A cached "this port is absent" is deliberately never reused — that direction is the dangerous one (it would report a port plugged in since the snapshot as missing, which would stop a transport from arming its presence check at all), so a cached miss always forces a fresh enumeration before answering.

Item 3 of the issue — the reader thread created per SCPI exchange — is not in this PR, so #491 stays open. It is not an idle cost (it only happens when you send a command), and removing it means reworking StreamMessageConsumer's thread lifecycle, including the stale-reader guard that stops two readers ever landing on one stream. That deserves its own change.

Verification

• 20 new tests. Both fixes are proven regression catchers by mutation: restoring the 100 ms poll fails 2, restoring the enumeration-first ordering fails 1, and serving cached negatives fails 3. Full suite green on net9.0 (3067 Core + 86 Mcp) and net10.0 (3067), 0 warnings.
• Bench, non-destructive, Nq1 fw 3.7.2 on /dev/cu.usbmodem1101. Harness alternating against an origin/main baseline worktree, 30 s idle with the transport connected and a producer running, 3 runs each: 230/255/257 ms CPU → 93/79/86 ms (7.7-8.6 → 2.6-4.0 ms per second) and 68.5 KiB allocated → 0, connection still healthy at the end of every run. One GetPortNames() costs 0.11 ms on this machine, so the enumeration alone was ~0.11 ms per device per second. Health checks after: serial discovery finds sn=9090539562006014104, 3 s @ 500 Hz on channels 0-2 → 1187 samples (this unit's known ~79 % clock ratio), SD storage query 7.80 GB, clean disconnect. No reboot, format, delete, SD:GET, firmware or LAN writes.
• The shared snapshot itself is not exercised on macOS (the device-node check answers first), so that half is covered by unit tests rather than by the bench.

Addresses items 1 and 2 of #491.

Not merging — this is for your review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Perf: eliminate idle device wakeups and cache serial-port enumeration

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Remove MessageProducer polling so idle connected devices generate no periodic wakeups.
• Reorder/optimize serial port presence probing with a shared, positive-only cache.
• Add regression tests to prevent reintroducing idle CPU and enumeration costs.

Diagram

graph TD
  A["SerialStreamTransport"] --> B["Presence probe"] --> C{ "Unix path?" }
  C -->|"Yes"| D["File.Exists(port)"] --> E["Present? (fast)"]
  C -->|"No / absent"| F["SerialPortNameSnapshot.Shared"] --> G["SerialPort.GetPortNames()"] --> H["Contains(port)"]
  I["MessageProducer"] --> J["ManualResetEventSlim.Wait()"] --> K["Drain queue"]

  subgraph Legend
    direction LR
    _svc(["Component"]) ~~~ _dec{"Decision"} ~~~ _ext["OS/.NET API"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Replace producer loop with Channel/BlockingCollection

• ➕ Built-in blocking semantics; less manual event/reset reasoning
• ➕ Potentially simpler shutdown and backpressure semantics
• ➖ Larger refactor with higher regression risk
• ➖ May require API changes and broader test updates

2. Event-driven port presence detection (OS notifications)

• ➕ Near-instant unplug detection without periodic polling
• ➕ Eliminates enumeration/stat costs entirely while idle
• ➖ Platform-specific complexity (WMI/udev/IOKit), harder to keep consistent
• ➖ More operational risk than a bounded, shared snapshot

Recommendation: The PR’s approach is a good, low-risk optimization: the producer already had correct signal paths, so removing the timeout eliminates wasted wakeups without changing behavior; likewise, a filesystem-first probe plus a shared, positive-only enumeration snapshot preserves correctness while making cost scale with time instead of device count. Consider larger refactors (Channels) or event-driven port detection only if future work demands even faster unplug detection or simpler concurrency primitives.

Files changed (5) +763 / -22

Enhancement (3) +206 / -22

MessageProducer.csRemove 100ms polling timeout; track wakeups for verification +31/-2

Remove 100ms polling timeout; track wakeups for verification

• Changes the background loop to wait indefinitely on the message-available event instead of polling every 100ms, eliminating idle wakeups. Adds an internal WakeCount metric (incremented on each wait return) to support regression testing of idle behavior.

src/Daqifi.Core/Communication/Producers/MessageProducer.cs

SerialPortNameSnapshot.csIntroduce shared cache for SerialPort.GetPortNames() with safe staleness rules +140/-0

Introduce shared cache for SerialPort.GetPortNames() with safe staleness rules

• Adds SerialPortNameSnapshot, a short-lived, locked cache around SerialPort.GetPortNames() with a default 1s duration and a process-wide Shared instance. Implements a positive-only reuse policy (cached hits allowed; cached misses force a fresh enumeration) and never caches failures, preserving 'not observed' vs 'absent' semantics.

src/Daqifi.Core/Communication/Transport/SerialPortNameSnapshot.cs

SerialStreamTransport.csMake port presence probe filesystem-first and use shared enumeration snapshot +35/-20

Make port presence probe filesystem-first and use shared enumeration snapshot

• Updates documentation to reflect worst-case unplug detection moving to ~3–4s due to snapshot lifetime. Reorders the probe to check File.Exists first for Unix device-node paths and only then consult the cached enumerator, and refactors IsPortEnumerated to accept an injectable snapshot for testing.

src/Daqifi.Core/Communication/Transport/SerialStreamTransport.cs

Tests (2) +557 / -0

MessageProducerIdleWakeupTests.csAdd regression tests ensuring idle MessageProducer never wakes +246/-0

Add regression tests ensuring idle MessageProducer never wakes

• Introduces tests that assert an idle producer accumulates zero wakeups, wakes once per send, and promptly stops from a parked wait. Includes targeted race tests to catch lost-wakeup scenarios that would strand messages with an unbounded wait.

src/Daqifi.Core.Tests/Communication/Producers/MessageProducerIdleWakeupTests.cs

SerialPortNameSnapshotTests.csAdd tests for shared serial port enumeration snapshot and probe ordering +311/-0

Add tests for shared serial port enumeration snapshot and probe ordering

• Adds coverage for snapshot caching (time window, concurrency, disabled caching), correct handling of cached misses (force refresh), and exception propagation rules. Also pins SerialStreamTransport.IsPortEnumerated ordering so Unix device-node presence avoids enumeration, while Windows-style names propagate enumeration failures.

src/Daqifi.Core.Tests/Communication/Transport/SerialPortNameSnapshotTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. WakeCount accumulates across restarts ✓ Resolved 🐞 Bug ≡ Correctness

Description

MessageProducer<T>.WakeCount is documented as counting wakeups “since Start()”, but _wakeCount is
never reset in Start(). If the same producer instance is stopped and restarted, WakeCount includes
prior runs and can mislead tests/diagnostics that rely on it.

Code

src/Daqifi.Core/Communication/Producers/MessageProducer.cs[R83-86]

+    /// A producer with nothing to send must never wake at all, so this stays at its value for as
+    /// long as the queue is empty and no stop has been requested.
+    /// </summary>
+    internal long WakeCount => Interlocked.Read(ref _wakeCount);

Relevance

●●● Strong

Team commonly fixes doc/contract mismatches and lifecycle state issues; WakeCount reset aligns with
“since Start()”.

PR-#260
PR-#321
PR-#248

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new XML doc states WakeCount is “since Start()”, but Start() does not reset the backing field,
so the counter persists across restarts of the same instance.

src/Daqifi.Core/Communication/Producers/MessageProducer.cs[81-108]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MessageProducer<T>.WakeCount` is described as tracking wakeups since `Start()`, but `_wakeCount` is never reset when starting a new run. If a `MessageProducer` instance is reused across stop/start cycles, `WakeCount` accumulates across runs.

### Issue Context
This PR introduces `_wakeCount` and `WakeCount` primarily for tests/diagnostics around idle CPU usage.

### Fix Focus Areas
- src/Daqifi.Core/Communication/Producers/MessageProducer.cs[81-108]

### Suggested fix
In `Start()`, reset `_wakeCount` before spinning up the background thread, e.g.:
- `Interlocked.Exchange(ref _wakeCount, 0);`

(Alternatively, if cumulative semantics are desired, update the XML doc to remove “since Start()”.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 10a4218

[tylerkron]

Qodo-clean, CI green — ready for review. (2 rounds on head 10a4218; the round-1 finding is fixed and its thread resolved. Bench re-run after that fix: 87 ms CPU / 0 KiB over 30 s idle connected, 1187 samples @ 500 Hz for 3 s on the Nq1.)