From 9694f115dd9f81163891b9a72ed6113fbc181bfa Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 6 Sep 2026 04:14:46 +0200 Subject: [PATCH] perf: planning a png or a gif stops coding the picture twice Planning drew the whole picture and compressed it only to learn its length, threw that away, and let the writer do it again. That was 38 to 53% of a PNG run. The ladder is walked largest rung first, so for any request comfortably above what the top rung encodes to, that rung is already the answer and the encode only confirms it. ladderCeiling is what "comfortably above" means, and planning takes the top rung without encoding when the request clears it. Everything near a rung boundary still encodes and still gets the exact number, so the answer never changes. The bytes are identical, and that was checked rather than reasoned: 147 combinations of size, seed, label and frame count - including sizes either side of the fast path threshold and either side of every rung - came out identical, refusals included. Measured with tools/probes/abcpu, order reversed, ranges disjoint: png, 300 files of 200 kB cpu 3797 -> 1859 ms wall 4734 -> 2580 ms gif, 200 files of 200 kB cpu 797 -> 453 ms wall 1232 -> 789 ms jpg is deliberately left alone. It writes its padding in FRONT of the picture, so the comment length has to be known before anything is encoded - which is exactly the number the fast path does not have. The only way round is moving the padding behind the picture, and that is a documented padding channel and a breaking change. Two guards, four mutations, all caught. One says the ceiling is safe - too low is the dangerous direction, because planning would then accept a size the writer has to refuse - and it reads the constant out of the source rather than copying it. The other says the fast path is actually taken: 50 plans allocate 29 kB against 2.4 MB for one write. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 19 +++ internal/format/gif/gif.go | 61 +++++++- internal/format/png/png.go | 83 +++++++++- internal/guard/ladderceiling_test.go | 219 +++++++++++++++++++++++++++ 4 files changed, 370 insertions(+), 12 deletions(-) create mode 100644 internal/guard/ladderceiling_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 843c0a8..5110d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,25 @@ because it turns other people's test suites red. ### Changed +- **Producing `.png` and `.gif` files is about twice as cheap.** Working out + what a file will contain used to draw the whole picture and compress it, only + to throw the result away and do it again when the file was actually written. + It now does that once. + + Measured on 300 files of 200 kB: `.png` takes **2.0 times less processor time + and 1.8 times less wall clock**. For `.gif`, 1.8 and 1.6. + + **The files are byte for byte identical.** This changes only how the work is + ordered, and it was checked that way - across sizes either side of every step + in the picture ladder, for several seeds, with the label on and off. + + A preview (`--dry-run`) of a large run gets the bigger share of this, since + previewing was almost entirely the work now removed. + + `.jpg` is unchanged and cannot get the same treatment: it writes its padding + in front of the picture, so it has to know how large the picture is before it + starts. + - **`verify` and `cleanup` read the files over several threads, so checking a large run is several times faster.** Nothing about what they report changes - the same differences, in the same order, with the same exit codes. diff --git a/internal/format/gif/gif.go b/internal/format/gif/gif.go index ee62db6..1fb4375 100644 --- a/internal/format/gif/gif.go +++ b/internal/format/gif/gif.go @@ -159,6 +159,11 @@ type memo struct { label string // body is the encoded picture up to but not including the trailer. body int64 + // bodyKnown says whether planning worked that out. For a request far + // above what the largest rung encodes to, the answer cannot change which + // picture is chosen, so planning skips the encoding and the writer fills + // this in. See ladderCeiling. + bodyKnown bool // payload is how many bytes of filler the comment carries, and blocks how // many sub blocks carry them. Both zero means no comment at all. payload int64 @@ -209,8 +214,14 @@ func (generator) Plan(r format.Request) (format.Plan, error) { }, } - if err := settlePadding(&m, r.Bytes, bare); err != nil { - return format.Plan{}, err + // With the body unknown the padding cannot be settled yet, and it does not + // need to be: the fast path in chooseSize already established there is room + // for a comment carrying whatever is left. The writer settles it once it + // has encoded, which it has to do anyway. + if m.bodyKnown { + if err := settlePadding(&m, r.Bytes, bare); err != nil { + return format.Plan{}, err + } } labelled := r.Label && imagelabel.Fits(w, len(label)) @@ -312,7 +323,17 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { if err := encode(holder, m); err != nil { return err } - if holder.written != m.body { + if !m.bodyKnown { + // Planning skipped the encoding, so this is where the exact size + // arrives and the padding gets settled - the same arithmetic planning + // would have done with the same number. + m.body = holder.written + if err := settlePadding(&m, p.Bytes, m.body+trailerSize); err != nil { + // Unreachable unless ladderCeiling is wrong, and then saying so + // beats writing a file of the wrong length. + return fmt.Errorf("gif: %w - ladderCeiling is wrong", err) + } + } else if holder.written != m.body { return fmt.Errorf("gif: the picture encoded to %d B where planning said %d B", holder.written, m.body) } if holder.tail[0] != 0x3B { @@ -368,6 +389,21 @@ func writeComment(ctx context.Context, w io.Writer, seed uint64, blocks, payload return err } +// ladderCeiling is the most the largest rung has ever been seen to encode to, +// with room to spare. It only ever decides that a request is far enough above +// the ladder that no search is needed, so being generous costs a few sizes +// their fast path and being wrong costs nothing silently - the writer refuses +// rather than producing a file of the wrong length. +// +// Measured 2026-09-06 at 640x480 over three seeds, the label both on and off, +// and one, three, ten and sixty frames: 54518 B at the smallest and 64020 B at +// the largest. Frames barely move it, about 150 B each, which is why this is +// one number rather than a function of the frame count. +// +// TestTheLadderCeilingIsAboveEveryPictureTheTopRungMakes sweeps it rather than +// trusting this comment. +const ladderCeiling = 98304 + // sizeLadder is tried from the largest down when the recipe names no picture // size, exactly as PNG does. The first rung that leaves a reachable remainder // wins, so a small file gets a small picture instead of being refused. @@ -402,10 +438,25 @@ func chooseSize(r format.Request, label string) (memo, error) { if err != nil { return memo{}, err } - m.body = body + m.body, m.bodyKnown = body, true return m, nil } + // Planning does not have to encode the picture to know which rung wins. + // The ladder is walked largest first, so for a request comfortably above + // what the largest rung encodes to, that rung is the answer and encoding + // only confirms it - at the cost of a whole encode thrown away so the + // writer can do it again (P7 in the 2026-09-05 performance review). + // + // A fast path, not a change of answer: it fires only where the rung is + // already settled, and the margin also guarantees the comment can carry + // whatever is left, so none of the refusals in settlePadding are reachable + // from here. + if r.Bytes >= ladderCeiling+trailerSize+smallestCarryingComment { + rung := sizeLadder[0] + return memo{width: rung[0], height: rung[1], frames: frames, seed: r.Seed, label: label}, nil + } + var smallest memo for _, rung := range sizeLadder { m := memo{width: rung[0], height: rung[1], frames: frames, seed: r.Seed, label: label} @@ -413,7 +464,7 @@ func chooseSize(r format.Request, label string) (memo, error) { if err != nil { return memo{}, err } - m.body = body + m.body, m.bodyKnown = body, true smallest = m bare := body + trailerSize diff --git a/internal/format/png/png.go b/internal/format/png/png.go index 5fde3a7..55ab2e6 100644 --- a/internal/format/png/png.go +++ b/internal/format/png/png.go @@ -118,10 +118,16 @@ type memo struct { width, height int seed uint64 label string - // body is the exact number of bytes the encoded picture takes before the - // closing chunk. Worked out during planning so that a size this format - // cannot reach is refused before any file exists. - body int64 + // body is the number of bytes the encoded picture takes before the closing + // chunk. Worked out during planning so that a size this format cannot + // reach is refused before any file exists. + // + // bodyKnown says whether it was worked out at all. For a request far above + // what the largest rung can encode to, the answer cannot change which + // picture is chosen, so planning skips the encoding and the writer - which + // has to encode anyway - fills both fields in. See ladderCeiling. + body int64 + bodyKnown bool // padData is how many bytes of padding the chunk carries. A negative // value means no chunk at all, which happens when the picture lands // exactly on the requested size. @@ -184,6 +190,14 @@ func (generator) Plan(r format.Request) (format.Plan, error) { bare := body + iendSize switch { + case !m.bodyKnown: + // The fast path in chooseSize already established that this request is + // far above the largest rung and that one chunk can carry the padding, + // so all three refusals below are unreachable and the only number still + // missing is how much padding there is. The writer settles that once it + // has encoded, which it has to do anyway. + m.withPad = true + case r.Bytes == bare: // The picture lands exactly on the requested size. No padding chunk. m.withPad = false @@ -263,7 +277,21 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { return err } - if holder.written != m.body { + if !m.bodyKnown { + // Planning skipped the encoding because the request was far above the + // largest rung, so this is where the exact size arrives. The padding is + // whatever is left, which is the same arithmetic planning would have + // done with the same number. + m.body = holder.written + m.padData = p.Bytes - m.body - iendSize - chunkOverhead + if m.padData < 0 || m.padData > maxChunkData { + // Unreachable unless ladderCeiling is wrong, and then it is better + // to say so than to write a file of the wrong length. + return fmt.Errorf( + "png: the picture encoded to %d B, which leaves %d B of padding for a %d B file - ladderCeiling is wrong", + m.body, m.padData, p.Bytes) + } + } else if holder.written != m.body { return fmt.Errorf("png: the picture encoded to %d B where planning said %d B", holder.written, m.body) } if string(holder.tail[4:8]) != "IEND" { @@ -280,6 +308,22 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { return err } +// ladderCeiling is the most the largest rung has ever been seen to encode to, +// with room to spare. It is only ever used to decide that a request is far +// enough above the ladder that no search is needed, so being generous costs a +// few sizes their fast path and being wrong costs nothing silently - a picture +// larger than this simply leaves less padding, and the writer would refuse +// rather than produce a wrong file. +// +// Measured 2026-09-06 over ten seeds with the label both on and off: 5456 B at +// the smallest and 5808 B at the largest, a spread of 352 B. The gradient +// compresses about 210 to 1, so the number is nowhere near the 1229280 B that +// an incompressible 640x480 picture would take. +// +// TestTheLadderCeilingIsAboveEveryPictureTheTopRungMakes sweeps it rather than +// trusting this comment. +const ladderCeiling = 16384 + // sizeLadder is tried from the largest down when the recipe names no picture // size. The first rung that leaves room for the padding chunk wins, so a // small file gets a small picture instead of being refused. @@ -321,10 +365,35 @@ func chooseSize(r format.Request, label string) (memo, error) { if err != nil { return memo{}, err } - m.body = body + m.body, m.bodyKnown = body, true return m, nil } + // Planning does not have to encode the picture to know which rung wins. + // + // The ladder is walked from the largest rung down and the first one that + // fits is taken, so for any request comfortably above what the largest rung + // encodes to, the answer is the largest rung and encoding only confirms it. + // That confirmation was 38 to 53% of a PNG run - a whole encode, thrown + // away, so that the writer could do it again (P7 in the 2026-09-05 + // performance review). + // + // This is a fast path and NOT a change of answer. It fires only where the + // rung is already settled, so the bytes are the ones the slow path below + // produces. Everything near a rung boundary still encodes and still gets + // the exact number. + // + // The second condition keeps the refusal above the chunk limit exact. + // Padding is r.Bytes minus the picture and the overheads, so it is largest + // when the picture is smallest, and a picture is never smaller than + // nothing. Bounding it that way costs a fallback to the slow path for a + // sliver of sizes just under two gigabytes and keeps the refusal honest. + if r.Bytes >= ladderCeiling+iendSize+chunkOverhead && + r.Bytes-iendSize-chunkOverhead <= maxChunkData { + rung := sizeLadder[0] + return memo{width: rung[0], height: rung[1], seed: r.Seed, label: label}, nil + } + var smallest memo for _, rung := range sizeLadder { m := memo{width: rung[0], height: rung[1], seed: r.Seed, label: label} @@ -332,7 +401,7 @@ func chooseSize(r format.Request, label string) (memo, error) { if err != nil { return memo{}, err } - m.body = body + m.body, m.bodyKnown = body, true smallest = m bare := body + iendSize diff --git a/internal/guard/ladderceiling_test.go b/internal/guard/ladderceiling_test.go new file mode 100644 index 0000000..b983ef4 --- /dev/null +++ b/internal/guard/ladderceiling_test.go @@ -0,0 +1,219 @@ +package guard + +import ( + "context" + "errors" + "fmt" + "go/ast" + "go/parser" + "go/token" + "runtime" + "strconv" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" +) + +// The ladder ceiling is above every picture the top rung can make. +// +// Planning used to encode the picture only to learn its length, throw that away +// and let the writer encode it again. That confirmation was 38 to 53% of a PNG +// run. It is skipped now whenever the request is far enough above the top rung +// that the search cannot come out any other way (P7 in the performance review +// of 2026-09-05), which measured 2.04x less processor time over 300 files. +// +// The whole of that rests on one claim: the top rung never encodes to more than +// ladderCeiling. If it did, planning would pick a rung whose picture does not +// leave room for the padding, and the writer would refuse a file that planning +// had already accepted - which breaks the promise that a preview gives the same +// verdict as the run. +// +// So this sweeps it. The number is read out of the source rather than copied +// here, because a copy is a second place to update and this repository has a +// guard whose whole job is finding numbers copied into prose. +func TestTheLadderCeilingIsAboveEveryPictureTheTopRungMakes(t *testing.T) { + // The top rung of every ladder, and what the format spends on top of the + // picture before any padding can start. Both are read from the source + // below, so this table only names what to sweep. + cases := []struct { + id string + file string + topWidth int + topHigh int + // extra sweeps the setting that moves the encoded size most: frames + // for GIF, nothing for PNG. + extra []map[string]string + }{ + { + id: "png", file: "../format/png/png.go", topWidth: 640, topHigh: 480, + extra: []map[string]string{{}}, + }, + { + id: "gif", file: "../format/gif/gif.go", topWidth: 640, topHigh: 480, + extra: []map[string]string{ + {"frames": "1"}, {"frames": "3"}, {"frames": "10"}, + {"frames": "30"}, {"frames": "60"}, + }, + }, + } + + // Seeds picked to spread the gradient offset, which is seed%256, rather + // than to look random. + seeds := []uint64{0, 1, 7, 42, 127, 128, 255, 256, 777, 7741, 99991, 123456} + + for _, c := range cases { + ceiling := ceilingFromSource(t, c.file) + d, err := format.Get(c.id) + if err != nil { + t.Fatalf("%s is not registered: %v", c.id, err) + } + + worst := int64(0) + var worstAt string + for _, extra := range c.extra { + for _, seed := range seeds { + for _, label := range []bool{true, false} { + props := map[string]string{ + "width": strconv.Itoa(c.topWidth), + "height": strconv.Itoa(c.topHigh), + } + for k, v := range extra { + props[k] = v + } + + // Asking for one byte at the top rung makes the format + // state its own floor, and that floor is the encoded + // picture plus whatever it always carries. Nothing else + // reports the encoded size from outside the package. + _, err := d.Generator.Plan(format.Request{ + Bytes: 1, Seed: seed, Label: label, Properties: props, + }) + var below *format.BelowMinimumError + if !errors.As(err, &below) { + t.Fatalf("%s at %dx%d refused one byte with %T rather than a BelowMinimumError: %v", + c.id, c.topWidth, c.topHigh, err, err) + } + if below.Minimum > worst { + worst = below.Minimum + worstAt = fmt.Sprintf("seed %d, label %v, %v", seed, label, extra) + } + } + } + } + + if worst > ceiling { + t.Errorf("%s: the top rung reaches %d B (%s) but ladderCeiling is %d.\n"+ + "Planning skips the encoding for any request above the ceiling and takes the top rung "+ + "on trust. A picture bigger than the ceiling leaves less room than planning assumed, so "+ + "the writer refuses a size planning accepted - a preview and a run disagreeing, which is "+ + "a row on the regression surface. Raise ladderCeiling in %s past %d.", + c.id, worst, worstAt, ceiling, c.file, worst) + } + t.Logf("%s: worst top rung %d B against a ceiling of %d, %.1fx of headroom (%s)", + c.id, worst, ceiling, float64(ceiling)/float64(worst), worstAt) + } +} + +// ceilingFromSource reads the ladderCeiling constant out of a format package, +// so the number lives in exactly one place. +func ceilingFromSource(t *testing.T, path string) int64 { + t.Helper() + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("reading %s: %v", path, err) + } + + var found int64 + seen := false + ast.Inspect(file, func(n ast.Node) bool { + spec, ok := n.(*ast.ValueSpec) + if !ok { + return true + } + for i, name := range spec.Names { + if name.Name != "ladderCeiling" || i >= len(spec.Values) { + continue + } + lit, ok := spec.Values[i].(*ast.BasicLit) + if !ok || lit.Kind != token.INT { + continue + } + v, err := strconv.ParseInt(lit.Value, 0, 64) + if err != nil { + continue + } + found, seen = v, true + } + return true + }) + if !seen { + t.Fatalf("%s has no ladderCeiling constant. If the fast path was removed, remove this guard "+ + "with it rather than leaving it passing on nothing.", path) + } + return found +} + +// Planning a picture format does not code the picture. +// +// The companion to the ceiling guard above: that one says the fast path is +// SAFE, this one says it is actually being taken. Without it the ceiling could +// be perfectly correct while planning encodes anyway, and the only symptom +// would be a preview of a large run costing what the run costs - which is the +// defect AVIF was rebuilt for and the same shape PNG and GIF carried until +// 2026-09-06. +// +// Fifty plans against one write, because a plan that codes is within a factor +// of one of a write and the gap is otherwise enormous. It asks the ALLOCATOR +// rather than the clock: allocation counts are deterministic, and a time based +// gate is flaky on a loaded runner. +func TestPlanningAPictureDoesNotCodeIt(t *testing.T) { + for _, id := range []string{"png", "gif"} { + t.Run(id, func(t *testing.T) { + d, err := format.Get(id) + if err != nil { + t.Fatal(err) + } + + const plans = 50 + // Comfortably above both ceilings, so the fast path is the one + // under test rather than the ladder walk. + const size = 300 << 10 + + runtime.GC() + var before runtime.MemStats + runtime.ReadMemStats(&before) + var last format.Plan + for i := 0; i < plans; i++ { + p, err := d.Generator.Plan(format.Request{Bytes: size, Seed: uint64(i), Label: true}) + if err != nil { + t.Fatalf("planning: %v", err) + } + last = p + } + var afterPlan runtime.MemStats + runtime.ReadMemStats(&afterPlan) + planning := int64(afterPlan.TotalAlloc - before.TotalAlloc) + + runtime.GC() + var beforeWrite runtime.MemStats + runtime.ReadMemStats(&beforeWrite) + if err := d.Generator.Write(context.Background(), &countingSink{}, last); err != nil { + t.Fatalf("writing: %v", err) + } + var afterWrite runtime.MemStats + runtime.ReadMemStats(&afterWrite) + writing := int64(afterWrite.TotalAlloc - beforeWrite.TotalAlloc) + + t.Logf("%d plans allocated %d B, one write allocated %d B", plans, planning, writing) + + if planning >= writing { + t.Errorf("%d plans of %s allocated %d B and one write allocated %d B.\n"+ + "Planning is coding the picture, which is what makes a preview of a large run cost what "+ + "the run costs. Planning takes the top rung from ladderCeiling and the encode belongs in Write.", + plans, id, planning, writing) + } + }) + } +}