FFT Window Threadsafety

[marcusmueller]

Description

I add locks around window setting/usage.

As a drive-by improvement, let's not use floor, ceil to implement (inherent) integer division semantics.

Related Issue

Fixes #7144

Which blocks/areas does this affect?

FFT

Testing Done

Checklist

• I have read the CONTRIBUTING document.
• I have squashed my commits to have one significant change per commit.
• I have signed my commits before making this PR
• My code follows the code style of this project. See GREP1.md.
• I have updated the documentation where necessary.
• I have added tests to cover my changes, and all previous tests pass.

[daniestevez]

The code looks good to me, but performance-wise I'm wondering if it wouldn't be better to get the window lock at the beginning of the work() function and hold it for the whole duration of work(). I'm slightly worried that if the FFT size is small, locking and unlocking the mutex for each individual FFT might introduce too much overhead.

[mbr0wn]

I tend to agree with @daniestevez, although I don't know about the performance overhead. But what if we flip it around: If we unlock sooner, we get better concurrency. To which I say "so what", what's the value of being able to call set_window() more often from a different thread. So if we lock for the duration of work(), we get maybe some, maybe very little performance gains, but we lose nothing of value.

[daniestevez]

@mbr0wn that's exactly what I had in mind. set_window() will typically be called infrequently, and it can wait until work() finishes to update the window.

[marcusmueller]

I'm fine either way, but I remember I had to be finer in locking granularity in one place, because otherwise I could deadlock our CI, then made it consistent in every place. Will have to check what the precise reason was, and whether it affects work, probably not. Anyways, currently no time, so this becomes a draft for now.