Skip to content

Commit 07d690f

Browse files
donislawdevclaude
andcommitted
targz: a compressed archive settles by giving back a whole block
CI found this, not the migration: three jobs went red on a change that had passed a targeted local run, and one of the three was a latent fault in compression rather than anything Go 1.27 did. A 256 KiB archive at compression best would not settle. Traced: the filler came down five or six bytes a round while the file sat at 262147 and 262148, wobbling by one. The filler is a tar entry and a tar entry is padded to a whole 512 byte block, so handing back fewer than 512 bytes changes WHICH bytes the archive carries and not HOW MANY - what comes out of gzip then moves by a byte either way. The walk assumed a slope of one and was stepping across a staircase. That was true from the day compression landed. Go 1.27 only moved which sizes sit on a step edge, which is why it looked like the compiler broke it. So it gives back a whole block on an overshoot now: that lands under the target, and the gzip extra field closes the rest exactly, because the field adds precisely what it is given. Two rounds instead of never. Checked over 90 consecutive sizes in four bands at all three levels. The other two were mine and both were the guards being right. sync.OnceValues in framing.go was new concurrency, which this project asks to be declared. Declaring it would have been the wrong answer: the registry works out this format's minimum during init, which asks for the framing anyway, so nothing was ever lazy about it. A package level variable is initialised once, before any goroutine exists, and needs no lock. The guard found real over-engineering rather than a missing entry. And a semicolon in a comment, which rule 13 does not take. Adds tools/probes/targzsettle, which is how the staircase was seen, and a mutation for the block step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f77d1fc commit 07d690f

2 files changed

Lines changed: 42 additions & 9 deletions

File tree

‎internal/format/targz/compress.go‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,34 @@ func nextFiller(target, got, filler int64) (next, extra int64, useExtra, done bo
126126
case deficit < 0 || (deficit > 0 && deficit < 2):
127127
// Overshot, or left a remainder the extra field cannot hold: it costs
128128
// two bytes before it holds anything. Give the filler back enough that
129-
// the field has room to work.
130-
return filler - (2 - deficit), 0, false, false
129+
// the field has room to work, and give back A WHOLE BLOCK MORE.
130+
//
131+
// The block is the point, and giving back only the overshoot is what
132+
// used to happen and does not converge. The filler is a tar entry, and
133+
// a tar entry is padded up to a whole 512 byte block - so handing back
134+
// fewer than 512 bytes changes WHICH bytes the archive carries and not
135+
// HOW MANY. What comes out the other side of gzip then wobbles by a
136+
// byte or so either way, and the walk spends its rounds stepping five
137+
// bytes at a time across a staircase, never landing.
138+
//
139+
// Measured 2026-09-01: a 256 KiB archive at compression best sat at
140+
// 262 147 and 262 148 B for eight rounds while the filler came down
141+
// from 258 481 to 258 447. This had been true all along and Go 1.27
142+
// only moved which sizes land on a step edge, so it looked like the
143+
// compiler broke it. Probe: tools/probes/targzsettle.
144+
//
145+
// Undershooting is safe and overshooting is not: the extra field adds
146+
// exactly what it is given, up to 65 531 B, so a round that lands under
147+
// the target finishes on the next pass. A whole block is well inside
148+
// that.
149+
next := filler - (2 - deficit) - tarBlock
150+
if next < 0 && filler > 0 {
151+
// Try with no filler at all before deciding the size is out of
152+
// reach. Only an archive that overshoots with nothing in it is
153+
// genuinely too small.
154+
next = 0
155+
}
156+
return next, 0, false, false
131157
case deficit == 0:
132158
// Landed without needing the field at all.
133159
return filler, 0, false, true

‎internal/format/targz/framing.go‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
// this compiler - the mutation runner was pointed at exactly that and it
3030
// cannot go red. What the measurement buys is the NEXT release, and no test
3131
// that runs today can demonstrate that. The mutations here cover the
32-
// arithmetic being wrong; they cannot cover it being right for the wrong
32+
// arithmetic being wrong. They cannot cover it being right for the wrong
3333
// reason. That is why this comment is long: it is the only thing standing
3434
// between a later reader and a tidy simplification back to the bug.
3535
package targz
@@ -39,7 +39,6 @@ import (
3939
"compress/gzip"
4040
"fmt"
4141
"io"
42-
"sync"
4342
)
4443

4544
// framing is what a level zero gzip stream costs beyond its content.
@@ -50,12 +49,20 @@ type framing struct {
5049
perBlock int64
5150
}
5251

53-
// gzipFraming measures the framing once and hands back the same answer after.
52+
// The framing, measured once before anything can ask for it.
5453
//
55-
// Lazy rather than at init because the answer is only needed when a size is
56-
// being worked out, and a package that measures something on every program
57-
// start makes every command pay for the one that needs it.
58-
var measuredFraming = sync.OnceValues(measureFraming)
54+
// A package level variable rather than a sync.Once, and the guard against
55+
// stray concurrency is what pointed that out. The first version reached for
56+
// sync.OnceValues to keep the measurement lazy, and lazy bought nothing here:
57+
// the registry works out this format's minimum during init, which asks for the
58+
// framing anyway, so every program was paying for it before main started
59+
// whichever way it was written. Go initialises a package variable exactly once
60+
// and before any goroutine exists, so there is nothing here for a lock to
61+
// protect.
62+
var framingValue, framingErr = measureFraming()
63+
64+
// measuredFraming hands back what was measured, in the shape the callers want.
65+
func measuredFraming() (framing, error) { return framingValue, framingErr }
5966

6067
// measureFraming works the two constants out from three compressions, and then
6168
// checks the model against a fourth.

0 commit comments

Comments
 (0)