Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After merging #21127, planning a FFT takes much longer #21163

Closed
giordano opened this issue Mar 26, 2017 · 2 comments
Closed

After merging #21127, planning a FFT takes much longer #21163

giordano opened this issue Mar 26, 2017 · 2 comments
Assignees
Labels
performance Must go faster regression Regression in behavior compared to a previous version

Comments

@giordano
Copy link
Contributor

giordano commented Mar 26, 2017

After merging #21127, there is a large performance regression when planning a FFT, this is particularly evident with the slow FFTW.MEASURE flag.

On commit d694548 (parent of the merge commit):

julia> FFTW.set_num_threads(4)

julia> v = Vector{Complex{Float64}}(nextpow2(100000));

julia> @time plan_fft(v, flags = FFTW.MEASURE);
  3.354884 seconds (334.41 k allocations: 17.760 MiB, 0.19% gc time)

julia> v = Vector{Complex{Float64}}(nextpow2(400000));

julia> @time plan_fft(v, flags = FFTW.MEASURE);
  2.556090 seconds (334.40 k allocations: 23.737 MiB, 0.23% gc time)

On commit 0b8c5ed (merge commit of #21127):

julia> FFTW.set_num_threads(4)

julia> v = Vector{Complex{Float64}}(nextpow2(100000));

julia> @time plan_fft(v, flags = FFTW.MEASURE);
 38.807557 seconds (334.40 k allocations: 17.735 MiB, 0.02% gc time)

julia> v = Vector{Complex{Float64}}(nextpow2(400000));

julia> @time plan_fft(v, flags = FFTW.MEASURE);
210.785749 seconds (380 allocations: 8.021 MiB, 0.01% gc time)
@tkelman tkelman added potential benchmark Could make a good benchmark in BaseBenchmarks performance Must go faster regression Regression in behavior compared to a previous version labels Mar 26, 2017
@stevengj
Copy link
Member

What's actually happening is that, after #21127, threads are not being used at all. This slows down the planner because there are more non-threaded plan possibilities than there are threaded-plan possibilities.

My bad, #21127 was the wrong solution and should be reverted/fixed.

@stevengj
Copy link
Member

I'll submit a new PR soon.

stevengj added a commit to stevengj/julia that referenced this issue Mar 27, 2017
stevengj added a commit that referenced this issue Mar 28, 2017
* updated fix for #19892; initialize FFTW threads the first time the planner is called (#21127 incorrectly prevented threads from being used at all)

* add test for #21163
@iamnapo iamnapo mentioned this issue Apr 5, 2017
tkelman pushed a commit that referenced this issue May 2, 2017
* updated fix for #19892; initialize FFTW threads the first time the planner is called (#21127 incorrectly prevented threads from being used at all)

* add test for #21163

(cherry picked from commit 378ed8a)
tkelman pushed a commit that referenced this issue May 3, 2017
* updated fix for #19892; initialize FFTW threads the first time the planner is called (#21127 incorrectly prevented threads from being used at all)

* add test for #21163

(cherry picked from commit 378ed8a)
@KristofferC KristofferC removed the potential benchmark Could make a good benchmark in BaseBenchmarks label Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants