feat!: default to Matrix buffers with optional backing array type#32
Merged
Conversation
SignalChannel, RechunkState, and the transforms (rechunk, mux, add) gain a
backing matrix type parameter M that defaults to Matrix{T}. Plain matrices can
now be put! directly; FixedSizeArrays is no longer required and becomes an
opt-in backing type, e.g. SignalChannel{T,N,FixedSizeMatrixDefault{T}}.
Benchmarks showed FixedSizeArrays gave no throughput benefit here: the channel
passes references and the bulk copies compile to memmove regardless of the
array type. FixedSizeArrays is therefore dropped from the package dependencies
and is now a test-only dependency.
BREAKING CHANGE: SignalChannel is now SignalChannel{T,N,M}; the default eltype
is Matrix{T} instead of FixedSizeMatrixDefault{T}. Code relying on take!
returning a FixedSizeMatrixDefault must construct channels with the explicit
backing type SignalChannel{T,N,FixedSizeMatrixDefault{T}} or adapt to Matrix{T}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 74.31% 75.00% +0.68%
==========================================
Files 8 8
Lines 545 552 +7
==========================================
+ Hits 405 414 +9
+ Misses 140 138 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…annel APIs AirspeedVelocity runs the PR head's benchmark scripts against both the PR and the baseline (main). Since the backing-array-type change is breaking, the head scripts must stay runnable on the old FixedSizeMatrixDefault-only API. Add a shared HAS_BACKING_TYPE_PARAM detector and an as_buffer helper in benchmarks.jl that feeds each package version its native zero-copy buffer type (Matrix on the new API, FixedSizeMatrixDefault on the old one), and relax buffer parameter annotations to AbstractMatrix. Verified the head scripts run against both the new working tree and the registered v6.2.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
The two-method put! (put!(::SignalChannel{T,N,M}, ::M) fast path plus an
AbstractMatrix fallback) failed to dispatch to the fast path: M is a diagonal
type variable shared with the channel type, so Julia did not consider the
::M method more specific than the ::AbstractMatrix fallback. Every put! hit the
fallback and copied the buffer via M(v), allocating one buffer per call and
regressing channel/rechunk throughput 2-500x (caught by the PR benchmark).
Replace it with a single put! method plus a convert-style _as_backing(::Type{M},
v) helper (modeled on Base.convert's (::Type{T}, ::T) / (::Type{T}, x) pair),
which dispatches the identity case reliably. Matching buffers are now stored by
reference (0 allocations); only genuinely foreign matrix types are constructed
into M. Add a regression test asserting take!(chan) === put! input.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PipeChannels 1.0.1 fixes a data-loss race where a consumer iterating/take!-ing could drop the final item when close() raced with the last put!. SignalChannels' zero-copy put! makes that window more likely to hit (e.g. flaky tee output), so require >= 1.0.1 (which excludes the fix-less 1.0.0). 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.
Summary
SignalChannelno longer mandatesFixedSizeArrays. Benchmarking showed FixedSizeArrays gave no throughput benefit here — the channel passes references, and the bulk copies compile tomemmoveregardless of array type (reference-passing is if anything marginally faster with plainMatrix).The buffer type is now a type parameter that defaults to
Matrix{T}, so plain matrices just work, while anyAbstractMatrix{T}(FixedSizeArrays, StaticArrays, …) remains available as an opt-in.Changes
SignalChannel{T,N}→SignalChannel{T,N,M}whereM<:AbstractMatrix{T}defaults toMatrix{T}.put!accepts anyAbstractMatrix{T}: stored by reference when it is alreadyM(zero-copy, the default case), otherwise constructed viaM(v). The old "FixedSizeMatrixDefault only" throwing guard is removed.RechunkState{T,N,M};rechunk,mux, andaddpropagate the input channel's backing type.FixedSizeArraysdropped from package dependencies → now test-only ([extras]). Users whoadd SignalChannelsno longer pull it in.test/backing_type.jlexercises the FSA opt-in end-to-end; existing tests migrated to the defaultMatrixAPI. Benchmarks migrated too.Opt-in example
Breaking change
The default
eltypeis nowMatrix{T}instead ofFixedSizeMatrixDefault{T}. Code relying ontake!returning aFixedSizeMatrixDefaultmust either construct channels with the explicit backing typeSignalChannel{T,N,FixedSizeMatrixDefault{T}}or adapt toMatrix{T}.Testing
Full suite passes locally (
julia --threads=4): SignalChannel 46, Backing matrix type parameter 15, Rechunk 113, Channel Utilities 43, Channel Combine 38, Stream Utilities 13, periodograms 8, SoapySDR Extension 6 (+2 pre-existing@test_broken).🤖 Generated with Claude Code