Skip to content

mel: precompute FFT twiddle factors and reuse scratch - #65

Open
haydonryan wants to merge 2 commits into
mudler:masterfrom
haydonryan:opt/mel-fft-precompute
Open

haydonryan wants to merge 2 commits into
mudler:masterfrom
haydonryan:opt/mel-fft-precompute

Conversation

@haydonryan

Copy link
Copy Markdown
Contributor

Replace the per-frame iterative radix-2 FFT's per-butterfly twiddle recomputation with a cached FftPlan (bit-reversal + per-stage twiddle table built once per FFT length, thread_local scratch, no per-frame allocation). The transform stays in double and the table is generated bit-identically to the old iterative complex-multiply, so the output is bit-identical.

Measured: ~8.5% faster mel frontend on the 110m (bit-identical).

Assisted-by: AI:DeepSeek-V4-Flash

Replace the per-frame iterative radix-2 FFT's per-butterfly twiddle
recomputation with a cached FftPlan (bit-reversal + per-stage twiddle
table built once per FFT length, thread_local scratch, no per-frame
allocation). The transform stays in double and the table is generated
bit-identically to the old iterative complex-multiply, so the output is
bit-identical.

Measured: ~8.5% faster mel frontend on the 110m (bit-identical).

Assisted-by: AI:DeepSeek-V4-Flash

@localai-org-maint-bot localai-org-maint-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rfft can be called with different FFT lengths on the same thread, but static thread_local FftPlan plan(n) permanently caches only the first length. The comment says “one plan per distinct n”, while the implementation has one plan total. A later call with another power-of-two length passes differently sized input to apply; it reads n_ values and silently produces the wrong transform (or reads past the shorter input). Please key the cache by n or rebuild when plan.n() != n, and add a regression test that calls rfft with two lengths in both orders on the same thread.

The optimization itself looks useful; this lifetime/cache mismatch is the blocker.

@haydonryan
haydonryan force-pushed the opt/mel-fft-precompute branch from 50cae1a to 5366723 Compare August 23, 2026 17:59
The FFT plan cache was a single thread_local FftPlan built for the FIRST
length seen, so a later rfft() call with a different power-of-two length
reused the wrong plan and transformed the wrong number of samples. Rebuild
the cached plan whenever n changes (zero steady-state cost: one int
compare). Add tests/test_fft.cpp which calls rfft with several lengths in
both orders on one thread to guard this.

Assisted-by: AI:DeepSeek-V4-Flash

@localai-org-maint-bot localai-org-maint-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache now rebuilds when the FFT length changes, and the same-thread regression covers multiple lengths in both directions. The focused and full project gates are green. This resolves my blocker. @mudler, this is good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants