diff --git a/CHANGELOG.md b/CHANGELOG.md index ddd8606..d83db14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,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) + } + }) + } +}