Include fewer files for faster builds - #5
Closed
tstenner wants to merge 3 commits into
Closed
Conversation
Add a header with often used forward declarations, Move factory out of sample class to make sample forward declarable
cboulay
added a commit
that referenced
this pull request
Jun 17, 2026
Addresses review findings on the synchronous (zero-copy) outlet: #1 Memory safety: the byte-swap path grew its scratch buffer with resize() inside the loop while holding pointers into it, so a reallocation dangled earlier const_buffers and sent freed memory to byte-swapped clients. The new sync_swap_buffers() reserves the exact size up front. #2 Correctness: buffers were classified by size, so an 8-byte sample (e.g. 2x int32 or 4x int16) was mistaken for a timestamp and reversed as one double instead of per channel value. Swapping is now driven by the sample tag, not buffer size, so it is correct for any sample_bytes. #3 Portability: the per-sample tag was stored in a uint64_t and sent via its first byte, which is 0x00 on a big-endian host. It is now a uint8_t (sync_ts_entry), so the correct tag byte goes on the wire everywhere. #4 Lifetime: sync mode now flushes on every push and ignores pushthrough. The gather buffers alias the caller's memory, so deferring the write (pushthrough == false) would retain dangling pointers once the caller reuses the buffer. #5/#7 Docs: document sync mode as single-producer (the push path is unsynchronized) and note that disconnected consumers are detected lazily. Also removes the unused scratch_ member and extracts the swap into an inline, unit-testable helper (src/sync_serialization.h). New internal tests drive that helper across the 8-byte-collision and many-sample (reallocation) cases that no little-endian integration test can reach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cboulay
added a commit
that referenced
this pull request
Jun 17, 2026
Addresses review findings on the synchronous (zero-copy) outlet: #1 Memory safety: the byte-swap path grew its scratch buffer with resize() inside the loop while holding pointers into it, so a reallocation dangled earlier const_buffers and sent freed memory to byte-swapped clients. The new sync_swap_buffers() reserves the exact size up front. #2 Correctness: buffers were classified by size, so an 8-byte sample (e.g. 2x int32 or 4x int16) was mistaken for a timestamp and reversed as one double instead of per channel value. Swapping is now driven by the sample tag, not buffer size, so it is correct for any sample_bytes. #3 Portability: the per-sample tag was stored in a uint64_t and sent via its first byte, which is 0x00 on a big-endian host. It is now a uint8_t (sync_ts_entry), so the correct tag byte goes on the wire everywhere. #4 Lifetime: sync mode now flushes on every push and ignores pushthrough. The gather buffers alias the caller's memory, so deferring the write (pushthrough == false) would retain dangling pointers once the caller reuses the buffer. #5/#7 Docs: document sync mode as single-producer (the push path is unsynchronized) and note that disconnected consumers are detected lazily. Also removes the unused scratch_ member and extracts the swap into an inline, unit-testable helper (src/sync_serialization.h). New internal tests drive that helper across the 8-byte-collision and many-sample (reallocation) cases that no little-endian integration test can reach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
This PR reduces the number of includes to reduce full compile times (because fewer headers are read for each file) and reduce incremental builds even more (because changes to a single header affects less object files) by forward declaring classes where possible and removing unneeded includes (checked with include what you use.
The typical speedup on an older laptop and a Raspberry Pi 3B for a full build is between 14-30%.