Disable FFT cache #12307
Disable FFT cache #12307
Conversation
|
Rather than disable it entirely, I'd rather at least make it opt-in. For example, start with a cache size / number of elements of zero, but allow changing this with some public function to be larger. @peterbell10 any ideas? |
I'm not necessarily disagreeing, but there may be unexpected complications: for example, |
|
Can we see the benchmark results for a range of different 1D fft sizes? I expect smaller transforms benefit the most anyway, so only caching transforms less than a fixed size might be better than disabling the cache completely. |
Sorry, I don't have an environment available at the moment where I can easily build |
|
I've benchmarked this and it does consistently slow down 1D transforms by 15-25% unfortunately. There also doesn't seem to be much of a length dependence here so setting a maximum plan size to cache would still significantly hurt 1D transform performance. User-configurable cache sizes could be a good option, and I think could be done without too much extra code. |
Agreed it is a bit of a can of worms, but if we can help speed up the code of 95% of users (I think some reasonable, enabled-by-default caching would do this) and give the remaining 5% a way to disable or configure things I think we should.
Sounds good to me. Not sure what the default cache size should be (unlimited as it is now? some percentage of the available memory?). But in any case, people wanting no caching would just set the cache size to zero, saving them from any potential can-of-worms (and known memory) problems. Some way to clear the cache but keep it the same size might be nice, though I guess setting it to zero then back to a non-zero value would accomplish this, so maybe an explicit clearing function is not needed. |
This is much less slowdown than I'd expected. My personal choice would definitely be to have no cache in this situation, given the tradeoff between additional complexity (both in the implementation and the user interface) and the potential gains. The decision is of course entirely up to you (i.e. the scipy developers), I just wanted to give my 2ct. (As an anecdote: before the C version of |
|
At the end of the day it comes down to the user's use case. If they are using multi-dimensional transforms then the cache is probably no more than a few % performance gain, so dropping the cache would be fine. If their use case is lots of repeated 1D transforms then a 15-25% performance gain is IMO still valuable. Though, obviously not as drastic as the likes of FFTW's plans. |
|
I think this fell victim to the Once |
|
Sounds good, thanks @mreineck
Maybe in one or two years - C++14 is okay now. Blockers for C++17 are documented in http://scipy.github.io/devdocs/dev/toolchain.html#c-language-standards |
|
Thanks for the link! |
That's a question for @peterbell10 I think. |
This addresses #12297 by disabling the internal plan cache of the FFT module completely.
While this may sound like a drastic solution, I'm pretty sure the performance consequences will be minor.
1D FFTs will become somewhat slower, but they have never been a strong point of
pocketfftanyway. Users who require really high 1D FFT performance are better served withpyFFTW.(Note, however, that for FFTs which are executed only once, the difference is pretty small.)
Multi-D FFTs will not suffer noticeably from a disabled plan cache, and I expect that most CPU time spent in scipy FFT calls will be inside multi-d transforms.
This is just a proposal of a simple fix for the problem raised in the issue. I'm sure there are more efficient approaches, but they will most likely be nontrivial, so maybe we should just apply this for the time being.