search

JuliaDSP / DSP.jl

Filter design, periodograms, window functions, and other digital signal processing functionality
https://docs.juliadsp.org/stable/contents/
Other
381 stars 109 forks source link

`filt` perf optimizations #516

Closed wheeheee closed 8 months ago

wheeheee commented 10 months ago
wheeheee commented 9 months ago

managed to squeeze performance of filt! to 2x on my device for _small_filt_fir, N < 18, by omitting inbounds.

wheeheee commented 9 months ago

Bump, couldn't figure out how to make BenchmarkPlots plot benchmarks in order, but the GitHub Actions tests take significantly less time to run (50 -> 30 minutes total usage). Some of this is maybe due to faster compilation for the generated functions. Tests in filt_stream.jl benefit quite a lot.

codecov-commenter commented 8 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (453c7e6) 97.46% compared to head (553b433) 97.49%.

Files Patch % Lines
src/Filters/remez_fir.jl 94.73% 1 Missing :warning:
src/dspbase.jl 98.14% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #516 +/- ## ========================================== + Coverage 97.46% 97.49% +0.03% ========================================== Files 18 18 Lines 3080 3078 -2 ========================================== - Hits 3002 3001 -1 + Misses 78 77 -1 ```

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

ViralBShah commented 8 months ago

@martinholters Ok to merge? Hope you don't mind the ping.

wheeheee commented 8 months ago

They were duplicates of each other except for the kernels so I edited the last one to accept all the FIRKernels and deleted the rest.

martinholters commented 8 months ago

Ah, yes, of course. I had missed the edited one.

ViralBShah commented 8 months ago

Merge? Also @wheeheee would you be ok to be invited to the organization to help maintain these packages?

wheeheee commented 8 months ago

I will say I don't have serious expertise in DSP, although I don't mind helping clean up issues / add documentation occasionally, and actually have a few more minor commits saved up to contribute.

Also, it might be good to do some benchmarking on other hardware for this commit. My laptop cpu (i5-1135g7) has avx512 instructions, which may possibly be the reason why the inbounds fiddling in _filt_fir! works.

ViralBShah commented 8 months ago

@wheeheee Thanks for helping with the maintenance here. Since we have approval here from @martinholters, it would be great to rebase and get this one merged.

wheeheee commented 8 months ago

Right, I think I'll just go ahead and merge to speed up CI. If the newest commits need modification, I'll open a PR later.

martinholters commented 8 months ago

Thanks @wheeheee, any help here is appreciated. And also thanks @ViralBShah for pushing this forward.

ViralBShah commented 8 months ago

Thanks @martinholters for all the work on this package, and @wheeheee for jumping in. We should see if there are other contributors who would like to step up as well. I'm just trying to help out - amazing how the small improvements over the last few days are making a big cosmetic difference at least.