Optimize performance of NeoViBus with multiple enhancements - #2098
Closed
pierreluctg wants to merge 10 commits into
Closed
Optimize performance of NeoViBus with multiple enhancements#2098pierreluctg wants to merge 10 commits into
pierreluctg wants to merge 10 commits into
Conversation
added 9 commits
September 1, 2026 10:24
…odule
perf(neovi): precompute base36 serial bounds in get_serial_number
Root cause:
`NeoViBus.get_serial_number()` recalculated `int("0A0000", 36)` and
`int("ZZZZZZ", 36)` on each invocation. Although each call is cheap,
this path is hit during config/device enumeration and repeated conversions
cause avoidable CPU work and temporary object churn.
Implemented solution:
- Add module-level constants:
- `SERIAL_BASE36_MIN = int("0A0000", 36)`
- `SERIAL_BASE36_MAX = int("ZZZZZZ", 36)`
- Reuse these constants in `get_serial_number()` comparisons.
Rationale vs alternatives:
This is a low-risk, low-effort micro-optimization with no behavior change.
Alternative options (e.g., caching per-device serial formatting) were not
selected because they add lifecycle complexity with little additional benefit.
Performance evidence:
- Measurement source: session microbenchmarking and profiling-focused review
of hot/cold paths in `neovi_bus.py`.
- Expected improvement: Low but measurable reduction in repeated numeric
conversion overhead in serial formatting path.
- This commit targets CPU overhead only; no protocol or I/O behavior changes.
Test methodology:
- Static code-path analysis + targeted microbenchmarks used for overall session.
- Functional validation performed via neoVI-focused test run in this session:
`python -m pytest test/test_neovi.py` -> passed.
Assumptions, limitations, risks:
- Assumes module import occurs once per process (normal case), so constants are
initialized once and reused.
- Speedup is small and mostly visible only under repeated enumeration calls.
Potential follow-ups:
- If needed, profile `find_devices` + serial formatting under repeated probing
workloads to quantify end-to-end impact more precisely.
Other identified optimizations not implemented in this commit:
- O(1) channel filtering via set membership.
- Event receipt lifecycle optimization.
- `_recv_internal` non-exception empty-path handling.
- `_ics_msg_to_message` constructor overhead reduction.
- `channel_to_netid` memoization.
Root cause: `_process_msg_queue()` previously checked membership with `channel in self.channels` where `self.channels` was a list. That made filtering O(N) per message and repeated global/attribute lookups inside the hot receive loop added overhead under high frame rates. Implemented solution: - Store `self.channels` as an immutable tuple. - Add `self._channel_set` and use it for O(1) membership checks. - Bind hot-loop references locally (`channel_set`, `rx_append`, `message_receipts`, `receive_own_messages`). - Reuse `status_bitfield` local instead of repeated field access. Rationale vs alternatives: This provides strong speedup with minimal risk and no behavior change. Alternative batching/vectorization approaches are not applicable here because message objects originate from the ICS driver API and must be handled in Python control flow. Performance evidence: - Session microbenchmark for membership checks: - list membership: 0.035455s - set membership: 0.011252s - speedup: ~3.15x (1,000,000 checks) - This commit also removes repeated lookup overhead in the same loop. Test methodology: - Synthetic timeit benchmark for list-vs-set membership representative of `_process_msg_queue()` filtering. - Functional validation in session with: `python -m pytest test/test_neovi.py` -> passed. Assumptions, limitations, risks: - Benefits scale with number of enabled channels and message rate. - End-to-end throughput still depends on ICS driver/hardware behavior. Potential follow-ups: - Run hardware-in-the-loop throughput profiling with real neoVI traffic mixes. Other identified optimizations not implemented in this commit: - `channel_to_netid` memoization. - Message constructor overhead reduction in `_ics_msg_to_message`. - `_recv_internal` empty-path exception removal. - Receipt event lifecycle optimization.
…channel name resolution perf(neovi): memoize channel_to_netid conversions Root cause: `channel_to_netid()` may be called repeatedly with the same channel names or IDs during repeated bus construction and config parsing, redoing identical integer conversion / attribute lookup work. Implemented solution: - Add `@functools.lru_cache(maxsize=128)` to `channel_to_netid()`. Rationale vs alternatives: Caching at this function boundary is the lowest-risk option: no behavior change, bounded memory, and no API changes. A larger or unbounded cache was not selected to avoid unnecessary memory growth. Performance evidence: - Evidence type: code-path and workload analysis from session. - Expected impact: Medium for repeated initialization/config loads with shared channel tokens; negligible for one-shot usage. Test methodology: - Functional verification via session neoVI test run: `python -m pytest test/test_neovi.py` -> passed. - Additional benchmarking in session focused on hot receive path; this commit targets constructor/config overhead. Assumptions, limitations, risks: - Cache keys are argument values; mixed types with same semantic meaning (e.g., `"1"` and `1`) are cached separately. - Small bounded cache may evict in very diverse channel-name workloads. Potential follow-ups: - If desired, profile repeated bus-init scenarios to quantify this in startup heavy workflows. Other identified optimizations not implemented in this commit: - Message conversion call overhead reduction. - `_recv_internal` empty-path optimization. - Receipt event lifecycle optimization.
Root cause: `message_receipts` used `defaultdict(Event)`, which can create event objects implicitly on missing-key access and adds extra dictionary-factory overhead. The timeout ACK path only needs explicit event objects per tracked transmit. Implemented solution: - Replace `defaultdict(Event)` with plain dict. - Allocate `Event()` explicitly only when `timeout != 0`. - Wait on local `receipt_event` reference. - Remove entry with `pop(receipt_key, None)` after wait to free memory safely. - Remove now-unused `defaultdict` import. Rationale vs alternatives: This keeps semantics explicit and minimizes accidental allocations while staying thread-safe at current call boundaries. Alternatives like global event pools were not selected due complexity and lifecycle risk. Performance evidence: - Evidence type: allocation-path analysis in send/ACK logic. - Expected impact: Medium for workloads that use timeout-based ACK waits and high transmit rates; lower for fire-and-forget sends. Test methodology: - Session functional validation: `python -m pytest test/test_neovi.py` -> passed. - Session microbenchmarks focused on receive hot-path; this commit targets send allocation/memory behavior. Assumptions, limitations, risks: - Behavior assumes receipt events are created only for messages that request ACK waits (`timeout != 0`), which matches existing logic. - Multi-threaded races are not expanded by this change but should still be validated with hardware-in-loop workloads. Potential follow-ups: - Add transmit-heavy benchmark with timeout ACK waits using real ICS device. Other identified optimizations not implemented in this commit: - `_ics_msg_to_message` constructor overhead reductions. - `_recv_internal` empty-buffer exception-path removal. - NetworkID cast micro-optimization.
Root cause: `_ics_msg_to_message()` created a new `functools.partial(Message, ...)` object for every received frame. That adds per-message allocation and call indirection in a high-frequency receive path. Implemented solution: - Remove `from functools import partial`. - Precompute frequently used fields into locals once per message. - Construct `Message(...)` directly (keyword form) in both FD and non-FD paths. Rationale vs alternatives: Direct constructor calls reduce call overhead without changing data flow. Larger structural changes (object pooling) were not selected due higher correctness risk and API-surface implications. Performance evidence: - Session microbenchmark (legacy partial-based pattern vs direct constructor pattern representative of this function): - legacy: 0.032160s - optimized: 0.022020s - speedup: ~1.46x (30,000 conversions) Test methodology: - Synthetic timeit benchmark using representative ICS-like message objects and equivalent legacy/new conversion logic. - Functional validation in session: `python -m pytest test/test_neovi.py` -> passed. Assumptions, limitations, risks: - Benchmark isolates Python conversion overhead and does not include hardware I/O latency from ICS drivers. - End-to-end gains depend on traffic profile and timestamp source mode. Potential follow-ups: - Validate throughput improvement with real neoVI hardware traffic replay. Other identified optimizations not implemented in this commit: - Positional-argument constructor calls for additional call overhead reduction. - `_recv_internal` empty-path exception removal. - NetworkID cast simplification in send path.
Root cause: Even after removing `partial`, `Message(...)` keyword argument parsing still adds overhead per received frame in `_ics_msg_to_message()`. Implemented solution: - Switch `Message` construction from keyword arguments to positional arguments in both FD and non-FD branches. - Preserve exact constructor order from `can.message.Message.__init__`. Rationale vs alternatives: This gives measurable constructor-call speedup with no algorithmic changes. Keeping keywords is more readable but slower in this hot loop. More aggressive alternatives (object pooling) were deferred due higher complexity/risk. Performance evidence: Session benchmark for `Message` constructor call style (200,000 iterations): - non-FD: kwargs=0.095052s, positional=0.055212s, speedup=1.722x - FD: kwargs=0.136029s, positional=0.099944s, speedup=1.361x Test methodology: - `timeit` benchmark directly constructing `can.Message` with equivalent values in keyword vs positional forms. - Functional validation in session: `python -m pytest test/test_neovi.py` -> passed. Assumptions, limitations, risks: - Positional form is more brittle if `Message.__init__` parameter order changes. - Added performance is Python-call overhead reduction; hardware I/O unchanged. Potential follow-ups: - Add a short comment near constructor call documenting positional order. Other identified optimizations not implemented in this commit: - `_recv_internal` empty-buffer exception-path removal. - NetworkID cast simplification in send.
Root cause: `_recv_internal()` relied on catching `IndexError` from `deque.popleft()` when no frame was available. In polling-heavy workloads, empty reads can be common, so exception construction/handling becomes an avoidable steady-state cost. Implemented solution: - Replace `try/except IndexError` with an explicit `if not self.rx_buffer` branch after queue processing. - Keep behavior identical: return `(None, False)` when no message is available. Rationale vs alternatives: Branch-based empty checks are cheaper than exception-driven control flow in normal operation and require minimal code change. Performance evidence: Session microbenchmark for empty recv control flow (500,000 iterations): - exception path: 0.061096s - branch path: 0.011707s - speedup: ~5.22x Test methodology: - Synthetic `timeit` benchmark comparing empty deque try/except vs pre-check. - Functional validation in session: `python -m pytest test/test_neovi.py` -> passed. Assumptions, limitations, risks: - Biggest win occurs when the receive loop polls frequently without data. - If traffic is always dense, relative benefit is lower. Potential follow-ups: - Measure in a real notifier/polling deployment with realistic idle/active mix. Other identified optimizations not implemented in this commit: - NetworkID cast simplification in send.
Root cause: `send()` converted expressions to `int(...)` even though bitwise operations on Python ints already produce ints. The extra calls add tiny but unnecessary overhead in a transmit hot path. Implemented solution: - Replace: `int(network_id & 0xFF), int((network_id >> 8) & 0xFF)` with: `network_id & 0xFF, (network_id >> 8) & 0xFF` Rationale vs alternatives: This is a no-risk micro-optimization and cleanup with identical semantics. Performance evidence: - Evidence type: Python operation semantics and call-overhead analysis. - Expected impact: Low but positive for high-frequency sends. Test methodology: - Session functional validation: `python -m pytest test/test_neovi.py` -> passed. Assumptions, limitations, risks: - `network_id` remains integer-valued as enforced by existing channel parsing. Potential follow-ups: - Include send-heavy throughput profiling to quantify aggregate impact. Other identified optimizations not implemented in this commit: - Additional zero-copy opportunities require ICS API support.
…and message processing
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of Changes
Performance Comparison vs.
mainBest Single Summary
For a receive-heavy mixed workload, the current neoVI implementation is approximately:
mainMeasured results:
This benchmark combines the primary receive-side optimizations:
Message(...)positional constructionScenario Breakdown
1. Mixed Receive Cycle
Representative batch processing, message conversion, and drain-loop workload.
Interpretation
This is the most representative overall benchmark for active receive workloads and is the best single number to summarize the impact of these changes.
2. Idle Polling / Empty Receive Path
Measures
_recv_internal()performance when no messages are available.Interpretation
Applications that poll frequently while idle or lightly loaded benefit significantly from this improvement.
The gain appears to come primarily from eliminating exception-driven control flow.
3. Channel Lookup / Initialization Path
Compares named channel resolution against the previous dynamic lookup behavior.
Interpretation
Approximately 18% faster channel-name resolution.
This mainly benefits repeated bus creation and configuration parsing and has minimal impact on steady-state runtime traffic.
4. Send Preparation Path
Measures Python-side transmit preparation only and does not include driver or hardware transmission latency.
Interpretation
Approximately 15% faster Python overhead in the measured send-preparation path.
Real-world end-to-end transmission latency improvements are likely smaller because driver and hardware latency dominate.
Practical Takeaways
Compared to
main, the current neoVI implementation is approximately:Limitations
These benchmarks measure Python-side overhead reductions only.
I was not able to benchmark:
As a result, these numbers should be interpreted as:
Benchmark Methodology
Focused microbenchmarks were run comparing
main-equivalent logic against the current implementation for:_process_msg_queue_ics_msg_to_message_recv_internalchannel_to_netidIn addition, the existing neoVI test suite was re-run:
python -m pytest "C:\Users\ptessie3\repos\EXTERNAL\python-can\test\test_neovi.py"Result:
Bottom Line
If you want a single number to quote:
Potential future follow-up:
examples/mainvs. current comparisons locallyRelated Issues / Pull Requests
Type of Change
Checklist
x`).
Additional Notes
Benchmarks were performed using focused Python microbenchmarks to evaluate relative overhead reductions versus
main. End-to-end hardware performance was not measured.