perf: one filler instead of thirteen, and eight bytes per draw instead of one - #56
Merged
Merged
Conversation
… instead of two Three findings from the performance report, and two of them came back with the report's own suggestion measured and refused. preflight asked the filesystem twice for every planned file. It reads the output directory once. A run of 100 000 files with --dry-run goes from 15.7-19.7 s to 0.49-0.56 s, order alternated - the check was 97% of it. Ten thousand names measured on their own: 1.937 s of stat calls against 8.9 ms for one listing. That is also a fix rather than only a speedup, and the guard for it says so. os.Stat follows a link, so a link pointing at nothing answered "no name here" and the run replaced it without a word. A directory ENTRY is what a taken name is, whatever it points at. With the old question put back, the new guard reports that the run went ahead over a name somebody else's link was holding. A directory that cannot be LISTED is still asked about file by file. Both systems allow write permission without read, a run into such a directory has always worked, and reading nothing there and calling it empty would let the run write over what is inside. That fallback has its own guard, which skips on Windows because denying a listing there needs an ACL. The plan ceiling forced a collection to take every reading, so a run of one kilobyte paid for two of them - measured with GODEBUG=gctrace=1, exactly two on every run however small. It asks /gc/heap/allocs:bytes first, at 251 ns against 519 us, and only collects when that says it might be over. The shortcut is sound by an inequality rather than by an estimate: the live heap cannot have grown by more than has been allocated. The report asked for /gc/heap/live:bytes and that metric is WRONG here. It reports the heap as of the last collection, and measured on 2026-09-05 all four existing ceiling guards stay green with it, because 25 MB of allocation makes the collector run on its own and the lagging reading catches up by luck. With the collector switched off the luck goes: a plan six times the ceiling is accepted. The new guard turns the collector off for exactly that reason. hashFile reads in 256 KiB pieces. The report asked for a 1 MB buffer through io.CopyBuffer and that does nothing at all: os.File implements io.WriterTo, so CopyBuffer hands it the whole job and throws the buffer away - 128 KiB, 256 KiB and 1 MiB with a plain file all take the same 167-172 ms that io.Copy takes. Hidden behind a reader that offers only Read, 256 KiB takes 161 ms against 199. The size is measured too: 64 KiB is 182 ms and nothing above a quarter of a megabyte can be told apart, so sixteen workers cost four megabytes. planChildren is sized up front, since the total is known from the groups. TotalBytes being walked twice is NOT done, and that is a measurement rather than an oversight: one walk over 100 000 planned files has a median of 0 s and a maximum of 541 us, so two of them cost half a millisecond of a nineteen second run. Widening a signature for that would be a change nothing can see. engine.go went past the length ceiling, and the guard asks for a split by what the parts do rather than for a bigger number - so preflight and the questions it asks about names are their own file now. Two ceilings came down with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d of one Thirteen packages each carried their own copy of the bulk random filler loop, in four different shapes. Measured over 64 MiB through a 32 KiB buffer, interleaved with the order reversed between repetitions: one draw per byte 182 MB/s zip, targz, wav eight via a temporary array 1468 MB/s bmp, gif, ico, opc, png, tiff eight via a shift loop 845 MB/s avif, jpg, jxl, webp one store into the buffer 2499 MB/s the shape they all use now All thirteen now call core.FillRandomBE or core.FillRandomLE. Two functions rather than one with a flag, because choosing the wrong byte order is not a style mistake - it silently rewrites every file a format has ever produced, and an argument would put that one typo away. BREAKING: zip, targz and wav files have different bytes. Their padding is where those formats spend almost the whole file, so almost every byte changes. Size, structure and readability are untouched. End to end on 64 MB, ranges disjoint: zip 2.81x, targz 2.74-3.32x. wav is in that list for uniformity and not for speed, by the owner's decision after the measurement: its padding is audio modulo the frame size, so exactly two bytes of a WAV differ and the time is unchanged. The other ten packages moved with no byte change at all - the shift loop IS little endian, which the performance report did not notice, so those four collapse to one store for free. Checked across 24 formats at five sizes and two seeds: three moved, twenty identical. Also closes a blind spot the golden set had. Forcing the filler to emit a constant moved 33 of the 54 pinned cases and not one WAV: wav_32kib lands on a size the audio fills exactly and never reaches the filler, so that path had no pinned witness. wav_with_the_padding_chunk is that witness. Guard: TestBulkRandomBytesComeFromOnePlace, two mutations, both caught. It names the one honest UintN caller as an exception and fails if that exception outlives its code. A third mutation proves the golden set covers the shared filler's short tail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Third chunk of the 2026-09-05 performance report:
P2andP12a.The report named four call sites. There are thirteen, in four shapes.
Measured over 64 MiB through a 32 KiB buffer, variants interleaved and the order reversed between repetitions:
zip,targz,wavP12a)bmp,gif,ico,opc,png,tiffavif,jpg,jxl,webpP12awas reported forpngalone - it is six packages. And the fourth shape was nobody's finding:byte(v >> (8*j))with ascendingjis little endian, so those four collapse to a singleLittleEndian.PutUint64, byte for byte identical, for free.Breaking:
zip,targzandwavbytes movePadding is where those formats spend almost the whole file. Size, structure and readability are untouched - only the padding content differs, so recorded hashes will not match.
End to end on 64 MB, order reversed, drift canary on every run, ranges disjoint:
zip2.81x,targz2.74-3.32x.wavgains nothing and is here by your decision after the measurement. Its padding isaudio % frameSize, so exactly two bytes of a WAV differ and the time is unchanged - the report grouped it with the other two because the loop looks identical, but the call site is not.Blast radius checked rather than assumed: 24 formats x 5 sizes x 2 seeds, exit code verified on every run. Three moved, twenty identical.
A blind spot in the golden set, found by probe
Forcing the filler to emit a constant moved 33 of the 54 pinned cases and not one WAV.
wav_32kiblands on a size the audio fills exactly, so it walks past the filler untouched - that path had no pinned witness at all while its bytes were being changed.wav_with_the_padding_chunkis that witness. Four cases repinned, one added, 55 total."Format has a golden value" is not "this path of the format has a golden value", and the list of case names does not show the difference.
Guard
TestBulkRandomBytesComeFromOnePlace- two mutations, both caught. It asks aboutUint64because that is what all thirteen reached for, names the one honestUintNcaller as an exception, and fails if that exception outlives its code. Its comment states what it does not see (a filler built onIntN,Uint32orFloat64, two of which are deliberately left alone ininternal/format/archive).A third mutation proves the golden set covers the shared filler's short tail.
Verification
go test -tags "$(cat .github/build-tags)" ./...- green, 82 packagespython tools/preflight.py --quick- all 12 checks passstaleness.py- 797 mutations, every pattern occurs exactly oncetry-named.pyon both affected guards - 12 mutations, all caughtNet: the tree lost 47 lines.
🤖 Generated with Claude Code