Performance improvements - #19
Open
Herrie82 wants to merge 3 commits into
Open
Conversation
The MLow encoder could not keep up with real time on an ARMv7 device (webOS TouchPad,
1.2GHz Scorpion): it encoded a 60ms frame in ~120ms, so a caller driving it from a 60ms
send ticker dropped every other frame and the peer heard the audio gated on/off like fast
morse code.
A CPU profile of MlowEncoder.Encode showed ~half of all time in math.Cos/math.Sin, called
from fftRec:
37.6% math.cos
10.5% math.sin
63.2% mlow.fftRec (cumulative)
The recursive mixed-radix DFT recomputed the twiddle factor exp(sign*2*pi*i*k*j/n) with a
cos/sin pair for every element of every transform. But the twiddles are constants that
depend only on (n, sign), and the analysis runs the FFT at a handful of fixed sizes each
frame. Memoize them: W[m] = exp(sign*2*pi*i*m/n) built once per (n,sign) into a table,
indexed by the reduced index (k*j) mod n. After the first frame every angle is a lookup.
Indexing by the reduced m in [0,n) is mathematically identical (cos/sin are 2*pi periodic)
and numerically cleaner than the original float32 k*j product, and it turns out to be
bit-identical on real input.
Benchmark (encode a 960-sample frame in a tight loop):
host x86-64: 8.07 -> 3.97 ms/frame (2.03x)
ARMv7 TouchPad: ~120 -> ~60 ms/frame (crossing into real time)
Correctness: bit-identical encoder output. Encoding a 3s speech sample before and after
yields the byte-identical 7055-byte MLow stream (md5 fdfe8362...), and the mlow package
test suite still passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
smplLPCAnalyzeWithF2 called buildDctTables() on every frame, which fills ~16 rows of cosines via genCosRow. Those tables depend only on nfft/order — they are constant across the whole call — so with the FFT twiddles now cached, genCosRow showed up as the next avoidable cost in the profile (~5%). Build the tables once behind a sync.Once and reuse the shared pointer. Benchmark (encode a 960-sample frame in a tight loop, host x86-64): 3.97 -> 3.80 ms/frame Correctness: bit-identical encoder output — the 3s speech sample still encodes to the same 7055-byte stream (md5 fdfe8362...), mlow tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With the twiddles cached, fftRec's own complex arithmetic (cpx.mul/cpx.add method calls)
became the top remaining hotspot in the encode profile (~23% flat). For a power-of-2 length
like the 512-point analysis FFT, smallestFactor is always 2, so the transform is entirely
radix-2. Special-case p==2 with an inlined butterfly: the q=0 twiddle is unity, so
out[k] = even[kmod] + W[k]*odd[kmod], written with direct float math instead of the cpx
methods, the q-loop, and the W[0] no-op multiply.
The operation order is identical to the generic loop it replaces, so the result is
bit-identical; the win is purely removing per-element call/branch overhead (and it matters
more on the in-order ARMv7 core than on x86, which already inlines the methods well).
Benchmark (encode a 960-sample frame in a tight loop, host x86-64):
3.80 -> 3.45 ms/frame
--- Cumulative result of the three MLow encoder optimizations in this branch ---
host x86-64: 8.07 -> 3.45 ms/frame (2.34x)
ARMv7 TouchPad: ~120 -> ~60 ms/frame from the twiddle cache alone, and ~40 ms/frame with
the inline butterfly plus GOMAXPROCS>=2 and a raised GC threshold on the
embedding side -- i.e. from 0.5x real time (frames dropped, gated "morse
code" audio) to ~1.5x real time (comfortable 60ms-frame headroom).
All three steps are bit-identical: a 3s speech sample encodes to the same 7055-byte MLow
stream at every stage (md5 fdfe8362...), and the mlow test suite passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
This PR does not deliver byte identical output. The twiddles() function changes the FFT semantics. In addition, the radix-2 path doesnt retain generic order since it uses a different calculation which leads to insignificant byte differences. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Working with a very old dual core, running into the limits, this will speed things up 2.3x for older socs while delivering byte identical output :)
Going from very choppy to almost realtime :)
Signed-off by: Herman van Hazendonk github.com@herrie.org