Skip to content

perf: 5.4x faster recompression (11.6x single-worker), byte-identical output - #1

Merged
Snesnopic merged 19 commits into
Snesnopic:mainfrom
W-Floyd:perf/huffman-region-search
Aug 1, 2026
Merged

perf: 5.4x faster recompression (11.6x single-worker), byte-identical output#1
Snesnopic merged 19 commits into
Snesnopic:mainfrom
W-Floyd:perf/huffman-region-search

Conversation

@W-Floyd

@W-Floyd W-Floyd commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Work on the hot paths, each measured on its own and each leaving output byte-identical. On a 2m38s 192 kbps CBR file (6071 frames, CRCs, window-switched granules), Apple M4 Max, Apple clang 21, -O3:

run before after
-z -t 1 (recompress, one worker) 2255 ms 194 ms 11.6×
-z (recompress, 16 cores) 218 ms 40.5 ms 5.4×
-t 1 (repack, one worker) 27.5 ms 24.5 ms 1.12×
no flags (repack, 16 cores) 27.4 ms 24.2 ms 1.13×

Both columns are medians of the two binaries run against each other in a rotating interleaved order, so the ratios are like for like; the after column won all eight pairs in every row.

Every step was A/B'd against its predecessor in a rotating interleaved order (a fixed order penalises whichever binary always runs last on a warming machine), and each figure below won every pair or nearly all.

The commits

perf(huffman): search only the region costs big_values can move (−72%) — find_best_config was 86% of a recompressing run's worker time. It built per-pair costs for all 288 pairs × 32 tables per granule, each through a Huffman-map lookup, then summed them into a per-pair prefix table: ~36 kB of heap traffic before the search proper began. But a pair's cost under a table follows from its clamped magnitudes alone (so: a table built once for the process), and every region boundary the side info can express is one of 23 scalefactor band boundaries (so: only the running totals at those boundaries are ever read). The accumulator now walks the pairs once in step with big_values, snapshotting each boundary as it passes; a span's cost is one 32-lane row subtracted from another. The count1 tail became a backward recurrence rather than a rescan per candidate.

perf(bitstream): one 64-bit access per field instead of a bit at a time (−28% / −20%) — both directions walked one bit per iteration, with a divide, a modulo, a bounds check and a read-modify-write each. Nothing in the bitstream is wider than 32 bits and a coefficient pair fits in 47, so one load or one accumulator window covers any field. Codewords resolve through an 8-bit lookup table per code table. This also fixed a latent bug: a field exactly filling the accumulator shifted it by 64, which is undefined rather than zero, and corrupted the granule.

perf(packer): stop copying and re-reading what is already in memory (−5.5% / −7.3%) — a freshly allocated, zeroed 576-entry vector twice per granule; a raw_bytes copy of every frame that nothing read; frames copied rather than moved; concatenations regrown as they went; a zero vector allocated per frame for padding. The input is also read once into memory instead of three or four stream reads per frame.

perf(packer): assemble the output in memory and write it once (−6.5%) — ≈18k stream writes plus two seeks for the Xing patch-up, on a file already wholly in memory.

perf(huffman): let the span search vectorise (−3.4% / −5.8%) — the 32-lane minimum compiled scalar because the cost was packed above the table index with a signed shift, which can overflow.

perf(huffman): hand-write the two cost kernels (−17.3% / −13.1%) — skip the escape row when it is all zeros (the common case), hold costs pre-scaled so the innermost loop needs no shift, and write both kernels as NEON intrinsics with the portable form kept behind -DMP3PACKER_NO_SIMD. The first two are −9.2%, the intrinsics a further −9.9%.

perf(huffman): enumerate candidates by shape, not by region counts (−42.8% / −22.0%) — the nested (region0_count, region1_count) loops evaluated 113 combinations per big_values, 4442 per granule, re-deriving the same region costs in most of them. Which shape a pair of counts produces depends only on how many boundaries lie below big_values, so the one-, two- and three-region cases can be enumerated directly — about 37 candidates per big_values — and the three-region case reuses a best-two-region cover settled once per boundary per granule.

perf(huffman): decode a count1 quadruple from one lookup (−2.9%) — a table indexed by the codeword's non-zero pattern together with the next four bits yields the whole quadruple, signs applied.

perf(packer): copy scalefactors rather than decode and re-emit them (−1.1% / −3.6%) — they are re-emitted exactly as they arrived, so only their total length matters; they were read field by field into a heap-allocated vector and written back one field at a time.

perf(huffman): skip the region a zero table codes, and bound it once (−2.1%) — table 0 codes nothing, so a region using it is a fill rather than a per-pair walk; and the spectrum end and the declared pair count were tested separately once per pair.

perf(packer): do not spawn workers when there is no search to share (−4.2% on a repack) — sixteen threads were created and joined to share work that, without -z, is a slice copy per frame.

A second pass, after comparing against a Go implementation of the same tool

Profiling both side by side attributed the remaining gap stage by stage, which turned up four more things:

perf(huffman): resolve a big-value pair from one ten-bit probe (−5.4% single-worker) — the probe returns the pair itself, both clamped magnitudes with the codeword length and each magnitude's sign-bit count packed into sixteen bits, so the symbol number never has to exist. Halving the entry pays for widening the table to ten bits, where nearly every codeword resolves in one lookup. The width was measured here, not inherited: eight, nine, eleven and twelve bits all cost 4.7–5.8%, each losing every interleaved pair, with a ten-against-ten control at +0.15%. A new test walks every prefix of every table from the root and holds the packed entry to it — it immediately found that codewords of exactly ten bits were being deferred to the tree walk.

perf(packer): describe the new reservoir instead of building it (−1.1% serial) — the layout pass concatenated every frame's data into a stream buffer that the write pass then copied out again. Deciding a frame's size and reservoir offset takes lengths only, so it records the pieces and the write pass reads them through a cursor.

perf(packer): let frames describe their bytes instead of copying them (−1.8% serial, −2.0% all-core recompress) — each frame copied its payload out of the file, and the layout pass copied all of those into the reservoir view: two copies of the whole audio and an allocation per frame, for bytes nothing writes to.

perf(packer): write new side info into the frame, not into an array beside it (−3.1% / −4.1% repack) — a parallel array of 200 bytes per frame, every byte a copy of what the frame already had. Also drops a per-granule vector of decoded scalefactors that nothing has read since scalefactors became a bit copy.

Tried from the same source and dropped: carrying the last non-zero coefficient from the search into the encoder, which is worth 1.8% in the Go implementation and −0.02% here over sixteen pairs. Its cost is the length of the trailing zero run, and Go's spectrum is eight bytes per coefficient where this one is two, so clang's vectorised scan had already made the walk nearly free.

Why the output is still byte-identical

Recompression is a search whose winner depends on tie-breaks, so "smaller file, same audio" is not a sufficient check — a tie broken differently silently changes the bytes. The candidate enumeration order, the 10000-per-pair penalty and the >= 10000 means impossible test are all preserved, so the winner is the old winner: cheapest total, then lowest big_values, then lowest region0_count/region1_count, then lowest table index.

Two behaviours that look like bugs are preserved deliberately, because output bytes depend on them:

  • A read running past the end of the data does not zero-fill; the bits it found stay at the top of the field and are shifted up again by the number it did not. A differential test of 3000 randomised trials against the byte-walking reader and writer pins this, along with mid-stream data() read-back.
  • Only the final 57 bits of the data are decoded a bit at a time, and scalefactors that reach past the end are still read field by field, since that is where the above applies.

A short read in the reader is likewise reproduced faithfully — the bytes that were there, zeros for the rest, and every later read failing too — so a truncated file still yields the same frames, and a leading ID3v2 tag claiming more than the file holds still yields none.

Checked after every step: byte-identical output over 8 files × {no flags, -z} × {-t 1, all cores}, covering MPEG-1 and MPEG-2 LSF, mono/stereo/joint, CBR and VBR, with and without CRCs, plus a Xing/LAME-tagged track. Also checked with the portable kernels built in.

Tried and dropped

  • Bounding each big_values candidate from below by the cheapest coding of its pairs and skipping candidates that cannot beat the incumbent: the bound is too loose to prune much, and the extra accumulation cost about 2%.
  • Batching the span kernel over rows sharing an endpoint: the rows are computed lazily here, so batching would compute rows no candidate asks about.
  • A branch-free big-values pair decode (magnitude/sign-count table, sign applied as an xor and an add): +1.7%. The branches it removes are ones clang had already compiled to conditional selects.
  • Hoisting the end-of-data check out of the pair loop: −0.3%, 4/8 pairs — noise.

One behaviour fix, and the first tests

fix(huffman): do not treat an uncodable tail as a free one. A count1 quadruple cannot start inside the last one, so a big_values that leaves one, two or three coefficients above coefficient 572 leaves them uncoded — but the cost model treated such a tail as free, which made those big_values look cheapest whenever the top of the spectrum was coded at all. The encoder then dropped the coefficients, the granule failed round-trip verification, and the whole frame was written verbatim: never wrong output, but recompression lost. A spectrum with a lone coefficient at 574 was coded as big_values 287 in 111 bits and dropped it; it is now 286 in 116 bits and reproduces exactly. None of the eight test files contains such a granule — 25,000 granules and not one, since encoders rarely code the top scalefactor band — so no output byte changes on any of them, but the input is reachable.

This also adds the first tests in the tree, both dependency-free programs wired into ctest behind MP3PACKERCPP_BUILD_TESTS (on by default for a top-level build):

  • coder_test holds the search to only ever returning a coding that reproduces the spectrum it was given, including the tail cases above.
  • bitstream_test is a differential test of the reader and writer against byte-at-a-time implementations of the same contract, over 3000 randomised trials. It pins the two behaviours output bytes depend on and that are expensive to debug from the far end — a read past the end of the data does not zero-fill, and data() may be called mid-stream — and it is what caught the shift-by-64 corruption.

Where the remaining time goes

After all this, a single-worker recompress splits roughly 38% search, 40% decode, 11% encode, and a plain repack is about a third raw read/write syscalls. The audio payload is still copied about five times between input file and output file (per-frame vector, pooled reservoir, per-frame result, concatenated output, output buffer); halving that is the obvious next step, and would mostly show up in the all-core figure, where the serial path is now over half the wall clock.

Two notes on API surface

  • Mp3Frame::raw_bytes is removed. It was write-only in-tree and reconstructible from the three fields beside it.
  • HuffmanOptimizer's coder now takes a Spectrum& (a std::array<int16_t, 576>) the caller owns, instead of returning a vector, so the buffer can be reused across granules.

Happy to split this into separate PRs, drop any individual commit, or re-measure on other hardware if that would help.

find_best_config was 86% of a recompressing run's worker time, and most of that
was work either repeated across candidates or never asked about.

Per-pair costs were built for all 288 pairs x 32 tables on every granule, each
entry through a Huffman-map lookup, then summed into a per-pair prefix table —
36 kB of heap traffic per granule before the search proper began. But a pair's
cost under a table follows from its clamped magnitudes alone, so it is a lookup
in a table built once for the process; and every region boundary the side info
can express is one of 23 scalefactor band boundaries, so only the running
per-table totals at those boundaries are ever read. The accumulator now walks the
pairs once in step with big_values, snapshotting each boundary as it passes, and a
span's cost is one 32-lane row subtracted from another.

Region costs are memoised by the shape of the span: spans below a boundary and
spans between two boundaries do not move with big_values and are settled once per
granule, so only the spans ending at big_values are recomputed per candidate. The
count1 tail is a backward recurrence over even positions instead of a rescan per
candidate. Band tables are static rather than a vector returned by value, which
cost three allocations per granule, and the search's working set is thread-local
rather than reallocated.

The candidate enumeration order, the 10000-per-pair penalty and the ">= 10000
means impossible" test are unchanged, so ties break exactly as before.

-72.0% on a single-worker recompress of a 6071-frame file (2233 -> 625 ms, 3.57x,
6/6 interleaved pairs); output byte-identical across 8 files x {"", -z} x {-t 1,
all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@W-Floyd

W-Floyd commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

All Claude, but hopefully the idea can prove useful. I was testing a reimplementation in Golang and noticed perf was definitely on the table.

W-Floyd and others added 4 commits July 29, 2026 20:49
Both directions of the bitstream walked one bit per iteration: a divide, a
modulo, a bounds check and a read-modify-write per bit. No MP3 field is wider
than 32 bits and a whole coefficient pair fits in 47, so a single 64-bit load or
accumulator window covers any of them.

The reader peeks a big-endian word at an explicit bit position, which also lets
the decoder carry its position in a register rather than round-tripping it through
the reader for every bit of every symbol. Its tail is mirrored into a zero-filled
buffer so a read near the end of the data is the same load as any other. The
writer holds pending bits in an accumulator that reaches the buffer eight bytes at
a time, and exposes pending/store/resume so the coder can keep the accumulator in
two locals for a whole granule instead of paying a load, a store and the
turnaround between them per field.

Decoding a codeword now starts from an 8-bit lookup table per code table, which
resolves the common short codewords in one step and hands back the tree node to
resume from otherwise. A whole pair — codeword, escape magnitudes and signs —
comes out of one peeked word, and a count1 quadruple with its signs likewise.
Encoding assembles a pair into one word and commits it in one step.

Two behaviours of the old code are preserved deliberately, because output bytes
depend on them:

  * A read running past the end of the data does not zero-fill. The bits it found
    stay at the top of the field and are shifted up again by the number it did
    not. A differential test of 3000 randomised trials against the byte-walking
    implementations pins this, along with mid-stream data() read-back.
  * The final 57 bits of the data, and only those, are decoded a bit at a time
    through the reader, since that is where the above applies and where a peeked
    word stops holding a whole symbol.

Fixed along the way: a field exactly filling the accumulator shifted it by 64,
which is undefined rather than zero, and corrupted the granule.

-28.4% single-worker and -19.9% all-core on a recompress of a 6071-frame file
(610 -> 437 ms and 79 -> 64 ms, 6/6 interleaved pairs each); output
byte-identical across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four things the hot paths were paying for and getting nothing from:

  * Every frame's coefficients came back from the decoder as a fresh
    std::vector<int16_t>, allocated and zeroed 576 entries at a time, twice per
    granule (once to decode, once to verify the re-encode). The coder now fills a
    Spectrum the caller owns — one pair of buffers per worker — and clears only
    the tail above the last coefficient it wrote, since everything below was
    written before it could be read.
  * Mp3Frame carried a raw_bytes copy of the whole frame, assembled for every
    frame and read by nobody: an allocation and a copy of the entire file's worth
    of audio, for a field that is trivially reconstructible from the three parts
    beside it.
  * Frames were copied out of the reader's optional into the frame vector, which
    duplicated each frame's payload and then destroyed the original. They are
    moved now.
  * The two whole-file concatenations regrew as they went, and each frame's
    trailing padding allocated a zero-filled vector to write it from. Both are
    sized up front; the padding comes from one shared static buffer.

The reader also reads the file once into memory and parses from there. A frame
needs three or four reads of a few bytes each, and going to the stream for them
cost more than the parsing did. A short read is reproduced faithfully — the bytes
that were there, zeros for the rest, and every later read failing too — so a
truncated file still yields the same frames, and a leading ID3v2 tag claiming more
than the file holds still yields none.

-5.5% on a repack and -7.3% on an all-core recompress of a 6071-frame file (6/6
interleaved pairs each); output byte-identical across 8 files x {"", -z} x {-t 1,
all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The output went out three stream writes per frame — header, side info, payload —
plus a fourth for padding, and the Xing patch-up then seeked back through them to
fix the byte count and the table of contents. On a 6071-frame file that is some
eighteen thousand writes and two seeks for a file that is already held in memory
in its entirety.

It is now assembled in one buffer, sized up front, patched in place, and written
in a single call. The padding is a resize rather than a write of a zero buffer.

-6.5% on a repack and -3.9% on an all-core recompress of a 6071-frame file (8/8
and 6/6 interleaved pairs); output byte-identical across 8 files x {"", -z} x
{-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cheapest table for a span is a minimum over 32 lanes, which should be four
vector minimums and a reduction. It was compiling to a scalar compare per table
because the cost was packed above the table index with a signed shift: that can
overflow, which is undefined, and is enough to stop the vectoriser cold. The same
minimum over unsigned values — costs are never negative, so the order is the same
— vectorises.

Tried and dropped in the same pass: bounding each big_values candidate from below
by the cheapest coding of its pairs, ignoring that a region shares one table, and
skipping candidates that could not beat the incumbent. The bound is too loose to
prune much and the extra accumulation cost about 2% — a net loss.

-3.4% single-worker and -5.8% all-core on a recompress of a 6071-frame file (8/8
and 6/6 interleaved pairs); output byte-identical across 8 files x {"", -z} x
{-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@W-Floyd W-Floyd changed the title perf(huffman): search only the region costs big_values can move perf: 5.3x faster recompression, 1.15x faster repack, byte-identical output Jul 30, 2026
The search's inner work is two kernels over the 32 code tables held side by side:
accumulating a pair's cost into the running per-table totals, and taking the
cheapest table for a span as a minimum over the difference of two rows. Both are
pure 32-lane vector work — eight vectors cover a whole row, and the only scalar
work is deriving two table addresses from a packed key — but the auto-vectoriser
was reloading the accumulators from memory every pair and adding a row of zeros
for nothing. Three changes, measured separately:

  * The escape row, which penalises tables whose linbits cannot reach the pair's
    magnitude, is all zeros unless the magnitude escapes at all — uncommon in real
    material. It is skipped rather than added.
  * Costs are held pre-scaled by 32 in the cost tables, so the low bits of a
    span's cost are already free to carry the table index that achieved it. That
    takes a shift per row out of the innermost loop, and the lane labels become a
    constant vector to OR in.
  * Both kernels are written as NEON intrinsics on arm64, with the portable form
    kept and buildable via -DMP3PACKER_NO_SIMD, which is what they are held
    against. The accumulators stay live in eight vectors for the whole loop, and
    the span kernel is eight subtracts, eight ORs, a tree of unsigned minimums and
    a horizontal fold.

On a single-worker recompress of a 6071-frame file the first two are worth -9.2%
and the intrinsics a further -9.9%, -17.3% together (8/8 interleaved pairs each);
-13.1% all-core, of which the intrinsics are -4.7%; -5.7% on a repack. Output
byte-identical across 8 files x {"", -z} x {-t 1, all cores}, and with the portable
kernels too.

Worth recording how nearly this went the other way: a first attempt measured the
intrinsics at +0.5% and concluded the compiler had already done the job. The
"portable" binary in that comparison was not portable — the guard had been patched
to `#if 0 && defined(__ARM_NEON) || defined(__ARM_NEON__)`, which parses as
`(0 && A) || B`, and Apple clang defines both macros. It was the vector build
measured against itself. Hence the build flag, so the comparison can be made
without editing the source.

Not adopted from the same source: batching the span kernel over the rows that
share an endpoint. The rows are computed lazily here, so batching would compute
some that no candidate asks about.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@W-Floyd W-Floyd changed the title perf: 5.3x faster recompression, 1.15x faster repack, byte-identical output perf: 6.2x faster recompression, byte-identical output Jul 30, 2026
W-Floyd and others added 6 commits July 29, 2026 21:46
Every candidate splits the pairs below big_values into one, two or three regions
at band boundaries. The nested (region0_count, region1_count) loops enumerated
113 combinations per big_values — 4442 per granule — and re-derived the same
region costs in most of them.

Which shape a (region0_count, region1_count) pair produces depends only on how
many boundaries lie below big_values. Call that nTail: because the boundaries are
sorted, "boundary[k] >= big_values" — the test that used to saturate both loops —
is exactly "k >= nTail". So the three shapes can be enumerated directly:

  * One region: the first region0 boundary at or beyond big_values swallows
    everything. One candidate.
  * Two regions: region1 runs from a boundary up to big_values, so each region0
    boundary contributes the smallest region1_count that reaches it. At most
    sixteen candidates, and the span each needs is one already computed for this
    big_values.
  * Three regions: regions 0 and 1 cover everything below their upper boundary,
    and neither moves with big_values, so their cheapest pairing is settled once
    per boundary per granule and only region2's tail is recomputed. At most
    twenty-one candidates, each a lookup and an add.

That is about 37 candidates per big_values instead of 113, with the two-region
prefix work amortised over the whole granule rather than repeated per candidate.

Ties still resolve as the nested loops did — lowest total, then lowest
region0_count, then lowest region1_count — which is why the prefix search prefers
the lowest region0 boundary on a tie, why the shapes are offered in that order,
and why the winner is chosen per big_values before being compared with the
incumbent.

-42.8% single-worker and -22.0% all-core on a recompress of a 6071-frame file (377
-> 215 ms and 58 -> 45 ms, 8/8 interleaved pairs each); output byte-identical
across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tail of a granule is coded as quadruples of -1, 0 and 1, and decoding one ran
a four-iteration loop with an unpredictable branch per position. The codeword's
non-zero pattern is four bits and the sign bits that follow it are at most four
more, so a table indexed by the pattern together with the next four bits — 256
entries — yields the whole quadruple, signs applied, in one lookup. A second
small table gives how many of those bits were really consumed.

Tried and dropped in the same pass: the same treatment for the big-values pairs —
a table of magnitudes and sign counts, with the sign applied branch-free as an xor
and an add. It measured +1.7% (1/8 pairs). The branches it removes are ones clang
had already compiled to conditional selects, so the table was a load for nothing.

-2.9% single-worker on a recompress of a 6071-frame file (7/8 interleaved pairs);
output byte-identical across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scalefactors are re-emitted exactly as they arrived — their values are not ours to
change, and their layout depends on scalefac_compress tables we only need to size,
never to decode. They were nonetheless read field by field into a heap-allocated
vector and written back one field at a time: an allocation and some seventy calls
through the bit machinery per granule, to reproduce bits that could have been
copied.

Their total length follows from scalefac_compress and, in granule 1, from the
scfsi bits that say a band group reuses granule 0's values and occupies no bits at
all. So the whole span is one bit copy, in 32-bit chunks.

The field-by-field path is kept for the case where the span reaches past the end of
the reservoir data, since what a read returns there depends on the width it was
made at, and the output bytes would differ. It also no longer allocates.

-1.1% single-worker and -3.6% all-core on a recompress of a 6071-frame file (10/10
and 9/10 interleaved pairs); output byte-identical across 8 files x {"", -z} x
{-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things in the big-values loop:

Table 0 codes nothing at all: every pair is a pair of zeros and no bits are
consumed. The loop was nonetheless peeking a word, looking up a codeword of zero
length and decoding a pair of zeros for each pair of the region. Region 2 uses
table 0 often on quiet material, so this is a fill rather than a walk.

A pair needs two coefficients, so the spectrum's own end bounds a region as much
as its declared pair count does, and the two were tested separately once per pair.
The first even position at or past 575 is 576, so folding them leaves one
comparison.

-2.1% single-worker and -0.8% all-core on a recompress of a 6071-frame file (10/10
and 7/10 interleaved pairs); output byte-identical across 8 files x {"", -z} x
{-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Left behind by the branch-free pair experiment recorded in
"decode a count1 quadruple from one lookup", which measured a loss and was
dropped. The table itself was never referenced after that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without -z a frame's worth of worker work is a slice copy, and sixteen threads
were being created and joined to share it — a fifth of the profile of a plain
repack. The work runs on the calling thread when there is only one worker or no
Huffman search to do.

-4.2% on an all-core repack of a 6071-frame file (7/10 interleaved pairs), level on
a single-worker one; output byte-identical across 8 files x {"", -z} x {-t 1, all
cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@W-Floyd W-Floyd changed the title perf: 6.2x faster recompression, byte-identical output perf: 5x faster recompression (10.9x single-worker), byte-identical output Jul 30, 2026
W-Floyd and others added 6 commits July 29, 2026 22:15
Two portability faults in the kernels added earlier, both of which would have
failed a build the CI matrix covers and neither of which this machine can hit:

  * The span kernel ends in a horizontal minimum, which has no 32-bit NEON
    encoding. __ARM_NEON is also defined on armv7, so the guard has to test for
    AArch64, and MSVC spells its NEON header differently in any case. Everything
    other than AArch64 with a GNU-style compiler now builds the portable form,
    which measured about 9% behind on a recompressing run.
  * The byte swap uses _byteswap_uint64 on MSVC, which needs <intrin.h>.

The portable form compiles clean under -Wall -Wextra and produces byte-identical
output across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A count1 quadruple cannot start inside the last one, so a big_values that leaves
one, two or three coefficients above coefficient 572 leaves them uncoded. The cost
model treated such a tail as costing nothing and being usable, which made those
big_values look cheapest whenever the top of the spectrum was coded at all — and
the encoder then dropped the coefficients.

The packer catches it: the granule fails round-trip verification and the whole
frame is written verbatim, losing its recompression rather than corrupting the
audio. So this was never a correctness fault in the output, only a granule the
search talked itself out of coding.

The three cases in the new coder test show it. A spectrum with a lone coefficient
at 574 was coded as big_values 287 in 111 bits, dropping it; it is now coded as
big_values 286 in 116 bits, and reproduces. A dense tail through 575 went from 279
and dropped to 280 and exact.

None of the eight test files contains such a granule — 25,000 granules and not one
— because encoders rarely code the top scalefactor band at all, and no output byte
changes on any of them. It is reachable input all the same.

Also adds the first tests in the tree, both dependency-free programs wired into
ctest behind MP3PACKERCPP_BUILD_TESTS (on by default for a top-level build):

  * coder_test holds the search to only ever returning a coding that reproduces
    the spectrum, including the three tail cases above.
  * bitstream_test is a differential test of the reader and writer against
    byte-at-a-time implementations of the same contract, over 3000 randomised
    trials. It covers the two behaviours output bytes depend on and that are
    expensive to debug from the far end: that a read past the end of the data does
    not zero-fill, and that data() may be called mid-stream. It is what caught a
    field exactly filling the accumulator being shifted by 64.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Decoding was the largest item left in the profile and its inner loop resolved a
codeword into a symbol, then split that symbol into two magnitudes and worked out
each one's sign bit — with an eight-bit probe that deferred to a tree walk for any
codeword longer than that.

The probe now returns the pair itself: both clamped magnitudes, the codeword length
and each magnitude's sign-bit count, packed into sixteen bits. The symbol number
never has to exist. Halving the entry is what pays for widening the table to ten
bits, where nearly every codeword on real material resolves in a single lookup and
the table is 2 kB. A deferred codeword is walked from the root rather than resumed
part way, since a resume position no longer fits in the entry and it is a codeword in
a hundred.

The width was measured, not inherited: against ten bits, eight costs 4.9%, nine
5.8%, eleven 4.7% and twelve 5.1%, each losing all eight interleaved pairs, with a
ten-against-ten control at +0.15% to show the harness is not tilted. The same idea in
the Go implementation of this tool, on this same machine, found eleven a tie with ten
— the probe tables are the same size there, so what differs is the rest of this
program's resident tables. An inherited constant would have been the wrong one.

The sign counts arriving with the pair also make the branch-free sign application
worthwhile, which it was not when it needed a second lookup of its own: an xor and an
add, with the bit advance a shift by the sign count.

A new test walks every prefix of every table one bit at a time from the root and holds
the packed entry to it — magnitudes, length, sign counts and the deferral flag alike.
It immediately found that codewords of exactly ten bits were being marked deferred,
since the builder tested for the probe running out before testing whether it had
landed on a leaf. Output was right and a tenth of the codewords in some tables took
the slow path.

-5.4% single-worker and -0.9% all-core on a recompress of a 6071-frame file (8/8 and
7/10 interleaved pairs; the all-core figure is diluted by the serial path, which this
does not touch). Output byte-identical across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The layout pass placed every frame's data into one stream buffer and the write pass
then copied that buffer into the output — a second copy of the whole audio, and an
allocation to hold it. The first pass never needed the bytes: deciding a frame's
size and its reservoir offset takes lengths only.

It now records the pieces — each frame's data, and the run of zeros at the end where
the reservoir cannot be read back — and the write pass reads them straight out of the
frames' own buffers. A frame's slot is a window over that sequence and generally
spans more than one piece, which is the whole point of a bit reservoir, so the read
side is a cursor rather than a slice.

-1.1% on a single-worker repack of a 6071-frame file (10/16 interleaved pairs), and
consistently signed but under the noise elsewhere: -0.1% all-core repack (8/10),
-0.4% all-core recompress (7/10). The copy removed is 3.7 MB of a 25 ms run, so about
1% is all it could have been worth. Output byte-identical across 8 files x {"", -z} x
{-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every frame copied its payload out of the file into a vector of its own, and the
layout pass then copied all of those into the reservoir view: two copies of the whole
audio and an allocation per frame, for bytes nothing ever writes to. The side info
went the same way, 32 bytes at a time.

The reader already holds the whole file, so a frame now records where its payload
starts, how long it is declared to be, and how much of that the file actually holds;
the side info sits inline, since 32 bytes is the largest there is. The reservoir view
is built in one pass out of the file, zero-filling any payload the file does not hold
in full — which is what a short read used to leave behind.

-1.8% on a single-worker repack (14/20 interleaved pairs) and -2.0% on an all-core
recompress (12/16) of a 6071-frame file. A first run at twelve pairs read +0.7% on the
repack, which sixteen more pairs did not support. Output byte-identical across 8 files
x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eside it

A parallel array held one SideInfo per frame — two hundred bytes each, every byte a
copy of something the frame already had. A worker owns its frame and builds the new
side info in place; a frame it gives up on keeps what it arrived with, which is what
the verbatim path reads, so nothing needs a second copy to fall back to.

GrChInfo also carried a vector of decoded scalefactors that nothing has read since
scalefactors became a bit copy. Dropping it takes another hundred bytes off every
frame, four vectors' worth.

-3.1% on a single-worker repack (11/16 interleaved pairs), -4.1% all-core (10/12) and
-1.4% on an all-core recompress (8/12) of a 6071-frame file. Output byte-identical
across 8 files x {"", -z} x {-t 1, all cores}.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@W-Floyd W-Floyd changed the title perf: 5x faster recompression (10.9x single-worker), byte-identical output perf: 5.6x faster recompression (12.5x single-worker), byte-identical output Jul 30, 2026
@W-Floyd W-Floyd changed the title perf: 5.6x faster recompression (12.5x single-worker), byte-identical output perf: 5.4x faster recompression (11.6x single-worker), byte-identical output Jul 30, 2026
@Snesnopic

Copy link
Copy Markdown
Owner

Hi, thanks for the PR! I've also seen your port in Golang which has some different design decisions and also optimizes MPEG2/2.5 files, I'd like to merge this PR and then work on integrating that part into mp3packercpp as well. The CIs aren't passing, I think you just need to add a #include <string> in mp3_reader.hpp. I will then run both your tests and some of my own, and then everything should be good

The header uses std::string& on line 27 but never includes <string>.
Some compilers/platforms relied on a transitive include that is no
longer guaranteed, causing 'string' in namespace 'std' errors on
GCC, Clang, and MSVC. This adds the explicit include for portability.

Signed-off-by: William Floyd <william.png2000@gmail.com>
@W-Floyd

W-Floyd commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

I want to be perfectly clear this is all Claude, so feel free to close/reimplement any way you like 😄

@Snesnopic
Snesnopic merged commit 5acbeac into Snesnopic:main Aug 1, 2026
12 checks passed
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