JuliaDSP / DSP.jl

Filter design, periodograms, window functions, and other digital signal processing functionality
https://docs.juliadsp.org/dev/
Other
374 stars 108 forks source link

replace splat with `Tuple` #513

Closed wheeheee closed 6 months ago

wheeheee commented 7 months ago

For a random matrix M of size (500, 500), allocations measured for unwrap(M; dims=1:2) drops from 2.2M+ to 250k, with a small speedup of about 30% on Windows, Julia 1.9.

This is achieved by changing every instance of splatting a vector into CartesianIndex{N} to Tuple.

wheeheee commented 7 months ago

I get a small (~3ms) speedup on a 50x50x50 array for using sum instead of the manually unrolled version.

codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (3e3524f) 97.29% compared to head (32a8298) 97.46%. Report is 7 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #513 +/- ## ========================================== + Coverage 97.29% 97.46% +0.17% ========================================== Files 18 18 Lines 3105 3080 -25 ========================================== - Hits 3021 3002 -19 + Misses 84 78 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

martinholters commented 7 months ago

I did a bit of benchmarking:

# Julia 1.0.5
julia> @btime unwrap(A; dims=1:2) setup=A=rand(500,500);
  154.448 ms (250021 allocations: 53.34 MiB) #master
  153.964 ms (250014 allocations: 53.34 MiB) #PR

julia> @btime unwrap(A; dims=1:3) setup=A=rand(50,50,50);
  118.314 ms (125025 allocations: 43.18 MiB) #master
  104.837 ms (235608 allocations: 49.93 MiB) #PR
# Julia 1.6.7
julia> @btime unwrap(A; dims=1:2) setup=A=rand(500,500);
  216.799 ms (2246030 allocations: 121.87 MiB) #master
  148.143 ms (250024 allocations: 53.34 MiB) #PR

julia> @btime unwrap(A; dims=1:3) setup=A=rand(50,50,50);
  111.778 ms (125034 allocations: 43.18 MiB) #master
  96.549 ms (125026 allocations: 43.18 MiB) #PR
# Julia 1.9.4
julia> @btime unwrap(A; dims=1:2) setup=A=rand(500,500);
  208.574 ms (2246032 allocations: 121.87 MiB) #master
  143.879 ms (250026 allocations: 53.34 MiB) #PR

julia> @btime unwrap(A; dims=1:3) setup=A=rand(50,50,50);
  110.831 ms (125036 allocations: 43.18 MiB) #master
  97.857 ms (125028 allocations: 43.18 MiB) #PR

So interestingly, between Julia v1.0.5 and v1.6.7, we've lost quite some performance for the 2d case which this PR successfully recovers almost without negative side-effects. There is a significant increase in the allocations for the 3d case on Julia v1.0.5, but a) it's a bit faster nevertheless and b) I don't think we should worry too much about Julia older than 1.6.