diff --git a/CHANGELOG.md b/CHANGELOG.md index 9645e0b0..a464dbdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,13 @@ because it turns other people's test suites red. ### Fixed +- **A run of many files no longer runs out of memory while it saves its + manifest.** The manifest was put together in memory in one piece, about + ten times its own size, after the last file. In a container with 1 GB a + run of 200 000 text files was killed at that moment and left its files + with no manifest, so `tfg cleanup` could not remove them. The manifest is + now written entry by entry, with the same bytes, and that run finishes. + Planning a large run also takes less memory. - **Ctrl+C stops a run that draws its sizes from a range.** With `--size-range`, every file was tried at its drawn size before anything else, and that part did not look at Ctrl+C - three thousand JPG files went on for diff --git a/internal/engine/engine.go b/internal/engine/engine.go index f4267528..13344200 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -450,6 +450,11 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF Remedy: core.Says("engine.NoOutDirFix", "Name a directory, for example ./fixtures")} } + // Worked out after the budget took its reference point, so the reserved + // list is weighed as part of the plan - which is what it is. Taken at the + // first file that passes its checks (plantarget.go), not here. + pl.reserve = planCapacity(targets, pl.budget) + for i := range targets { t := &targets[i] diff --git a/internal/engine/planmemory.go b/internal/engine/planmemory.go index 265e4012..3619b286 100644 --- a/internal/engine/planmemory.go +++ b/internal/engine/planmemory.go @@ -1,12 +1,47 @@ package engine import ( + "reflect" "runtime" "runtime/metrics" "github.com/donislawdev/TestingFilesGenerator/internal/core" ) +// planCapacity is how long a list to reserve for the plan. It is worked out +// before the first target is walked and taken at the first file that passes +// its checks (plantarget.go says why then). +// +// Reserved rather than grown since 2026-11-02. Grown by append, a plan of a +// million text files took 1.9-2.2 GB from the system for 0.94 GB of plan, +// because every doubling leaves the last array to the collector. Reserved, it +// took 1.2 GB and five collections instead of fourteen - the plansize probe, +// two runs each, interleaved (docs/FILE-CEILING-2026-11-02.md section 2). +// +// Never more than the run may hold. A count past the file ceiling is not +// reserved at all: the loop refuses it at the target that crosses the ceiling, +// and reserving it first would be the very allocation the ceiling is there to +// prevent. Under a memory setting the reservation stops where the bare list +// would fill the plan's ceiling, so the reservation alone is never what takes +// a run past the memory its person allowed - a plan that needs more than that +// is refused by account as the list grows, as it always was. +func planCapacity(targets []Target, budget *planMemory) int { + n := 0 + for i := range targets { + n += len(targets[i].Sizes) + if n > core.MaxFilesPerRun { + return 0 + } + } + if budget == nil { + return n + } + if fit := budget.ceiling / uint64(reflect.TypeFor[PlannedFile]().Size()); uint64(n) > fit { + return int(fit) + } + return n +} + // The ceiling on what a plan is allowed to cost. // // core.MaxFilesPerRun counts files, and the number it settled on was justified diff --git a/internal/engine/plantarget.go b/internal/engine/plantarget.go index 0ae79a1d..d3b85c3c 100644 --- a/internal/engine/plantarget.go +++ b/internal/engine/plantarget.go @@ -25,6 +25,9 @@ type planning struct { names map[string]nameOwner totalBytes int64 budget *planMemory + // reserve is how long a list the plan takes at its first file - see + // planCapacity for how long, and startList for when. + reserve int } // files plans every file of one target and adds them to the run. @@ -119,6 +122,11 @@ func (pl *planning) files(ctx context.Context, t *Target, desc format.Descriptor Because: core.SaidOf(err)} } + // Here, after every check above, and not before the first target - + // see startList for why. + if err := pl.startList(ctx); err != nil { + return err + } pl.out = append(pl.out, PlannedFile{ ID: fmt.Sprintf("f_%04d", len(pl.out)+1), Target: t, @@ -142,3 +150,26 @@ func (pl *planning) files(ctx context.Context, t *Target, desc format.Descriptor } return nil } + +// startList takes the list of the plan at its first file, as long as +// planCapacity said, and does nothing at every file after that. +// +// Not before the first target. A run refused at its first file - a size its +// format cannot make - is refused in tens of megabytes, and reserving first +// took a million such files to 450 MB before saying no (review of #212, +// measured 2026-11-02: 27-30 MB against 441-450 MB, two runs each). Ctrl+C is +// asked first for the same reason: a run that is being stopped is not given +// the largest allocation it would make. +// +// A method rather than a block inside files, because there it was the +// fiftieth function nested three deep, and that ceiling is a ratchet. +func (pl *planning) startList(ctx context.Context) error { + if pl.out != nil { + return nil + } + if err := ctx.Err(); err != nil { + return err + } + pl.out = make([]PlannedFile, 0, max(1, pl.reserve)) + return nil +} diff --git a/internal/guard/manifestreach_test.go b/internal/guard/manifestreach_test.go index b33b4c49..b7ca6f99 100644 --- a/internal/guard/manifestreach_test.go +++ b/internal/guard/manifestreach_test.go @@ -70,17 +70,11 @@ func TestAnOrdinaryRunSaysNothingAboutTheManifestCeiling(t *testing.T) { } } -// The length worked out for a manifest is the length Encode writes, to the -// byte. -// -// This replaces a guard that held an estimate to "a quarter either way", and -// a quarter was wide enough to let a constant 6% low for text and half of what -// an e-mail entry costs stay green while a run of 20 500 files went unwarned. -// The answer is counted entry by entry now, so it is held to the byte: a -// document of no entries, one entry, and many unlike each other - notes, a -// failure, words outside ASCII, characters JSON escapes, properties nested -// several deep. -func TestTheManifestSizeIsTheLengthOfTheManifest(t *testing.T) { +// manifestSamples are the documents both guards of the manifest's layout ask +// about - its counted length and its streamed bytes - so neither keeps a +// second copy of the cases: none, one, and many entries with text that +// encoding/json escapes, nested properties, notes and damage. +func manifestSamples() map[string]*manifest.Manifest { documents := map[string]*manifest.Manifest{"no entries": newDocument()} one := newDocument() @@ -99,8 +93,21 @@ func TestTheManifestSizeIsTheLengthOfTheManifest(t *testing.T) { many.Add(manifest.File{ID: "failed", Path: "failed.png", Name: "failed.png", Format: "png", Failed: true, Error: "a generator stopped", Damage: []manifest.Damage{{Type: "zero-head", Settings: map[string]string{"bytes": "4"}}}}) documents["many entries"] = many + return documents +} - for name, m := range documents { +// The length worked out for a manifest is the length Encode writes, to the +// byte. +// +// This replaces a guard that held an estimate to "a quarter either way", and +// a quarter was wide enough to let a constant 6% low for text and half of what +// an e-mail entry costs stay green while a run of 20 500 files went unwarned. +// The answer is counted entry by entry now, so it is held to the byte: a +// document of no entries, one entry, and many unlike each other - notes, a +// failure, words outside ASCII, characters JSON escapes, properties nested +// several deep. +func TestTheManifestSizeIsTheLengthOfTheManifest(t *testing.T) { + for name, m := range manifestSamples() { counted, err := m.EncodedBytes() if err != nil { t.Fatalf("%s: counting: %v", name, err) diff --git a/internal/guard/manifeststream_test.go b/internal/guard/manifeststream_test.go new file mode 100644 index 00000000..0003892a --- /dev/null +++ b/internal/guard/manifeststream_test.go @@ -0,0 +1,129 @@ +package guard + +import ( + "bytes" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// The manifest is written an entry at a time, and the bytes are still the ones +// encoding/json writes. +// +// Until 2026-11-02 Encode was one call into encoding/json, which builds the +// whole document in memory before its first write and indents it into a second +// copy while every entry is still held. At 100 000 text files saving the +// manifest peaked at 9.3 KB a file against 4.0 KB for writing every file, and a +// run of 200 000 in a container of 1 GB was killed after its last file with no +// manifest written (docs/FILE-CEILING-2026-11-02.md section 8). Encode now +// streams. Two things can go wrong with that and each half of this guard is +// about one of them. +// +// The bytes. verify, cleanup, the exports and every reader somebody wrote +// against a manifest read these bytes, and the streamed layout is assembled by +// hand. So it is compared with encoding/json's own rendering of the same value +// - not with size.go, which counts the same hand made layout and would agree +// with a mistake both made the same way. +func TestTheManifestIsWrittenAsEncodingJSONWritesIt(t *testing.T) { + documents := manifestSamples() + documents["a list that was never made"] = neverListed(t) + documents["two thousand entries"] = manyEntries(2000) + for name, m := range documents { + var streamed bytes.Buffer + if err := m.Encode(&streamed); err != nil { + t.Fatalf("%s: Encode: %v", name, err) + } + if want := encodingJSON(t, m); !bytes.Equal(streamed.Bytes(), want) { + at := firstByteApart(streamed.Bytes(), want) + t.Errorf("%s: Encode wrote %d B and encoding/json writes %d B, apart from byte %d:\n got: %q\nwant: %q", + name, streamed.Len(), len(want), at, bytesAround(streamed.Bytes(), at), bytesAround(want, at)) + } + } +} + +// And the document is never handed over whole. This is the half that says the +// streaming is there at all: going back to json.NewEncoder writes the same +// bytes in ONE write, the half above stays green, and this one turns red. +func TestTheManifestIsNotHeldWholeToBeWritten(t *testing.T) { + m := manyEntries(2000) + var w writeSizes + if err := m.Encode(&w); err != nil { + t.Fatalf("Encode: %v", err) + } + // The question only means something for a document many writes long, so + // that is asserted rather than assumed. + if w.total < 512<<10 { + t.Fatalf("the document of two thousand entries is only %d B - too short to tell a whole write from a streamed one", w.total) + } + if w.largest*8 > w.total { + t.Errorf("a manifest of %d B reached its writer in %d writes, the largest %d B. "+ + "Written whole, the document sits in memory beside every entry it was made from", w.total, w.writes, w.largest) + } +} + +// writeSizes counts what a writer was handed and the largest single piece. +type writeSizes struct{ total, writes, largest int } + +func (w *writeSizes) Write(p []byte) (int, error) { + w.total += len(p) + w.writes++ + w.largest = max(w.largest, len(p)) + return len(p), nil +} + +// encodingJSON is the manifest as encoding/json renders it in one piece, with +// the indent Encode has always used. +func encodingJSON(t *testing.T, m *manifest.Manifest) []byte { + t.Helper() + var b bytes.Buffer + enc := json.NewEncoder(&b) + enc.SetIndent("", " ") + if err := enc.Encode(m); err != nil { + t.Fatalf("encoding/json: %v", err) + } + return b.Bytes() +} + +// neverListed is a manifest read from a document with no list of files at +// all, so its list is nil - encoded as null rather than [], and a streamed +// layout that assumed a list would write the wrong one. +func neverListed(t *testing.T) *manifest.Manifest { + t.Helper() + var m manifest.Manifest + if err := json.Unmarshal([]byte(`{"manifest_version":"1.0"}`), &m); err != nil { + t.Fatalf("decoding: %v", err) + } + if m.Files != nil { + t.Fatalf("the list came back as %v rather than nil, so this case is not the one it names", m.Files) + } + return &m +} + +// manyEntries is a manifest long enough that a whole write and a streamed one +// cannot be confused, with entries that carry escaped text and nested values. +func manyEntries(n int) *manifest.Manifest { + m := newDocument() + for i := range n { + name := fmt.Sprintf("files_%04d <%d> & \"x\".txt", i+1, i) + m.Add(manifest.File{ID: fmt.Sprintf("f_%04d", i+1), Path: name, Name: name, Format: "txt", Bytes: int64(i), + Hashes: manifest.Hashes{SHA256: strings.Repeat("cd", 32)}, + Properties: map[string]any{"line": "a\u2028b", "depth": i % 3, "nested": map[string]any{"k": []any{1, "two"}}}}) + } + return m +} + +func firstByteApart(a, b []byte) int { + for i := range min(len(a), len(b)) { + if a[i] != b[i] { + return i + } + } + return min(len(a), len(b)) +} + +func bytesAround(b []byte, at int) []byte { + return b[max(0, at-40):min(len(b), at+40)] +} diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index 8ada20f8..65c6f68e 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -15876,8 +15876,8 @@ # the source sees it, which is why that walk exists. "the engine makes a context from nothing in a place nobody listed", "internal/engine/plantarget.go", - "if err := ctx.Err(); err != nil {", - "if err := context.Background().Err(); err != nil {", + "// thousand pictures is the case this is for.\n\t\tif err := ctx.Err(); err != nil {", + "// thousand pictures is the case this is for.\n\t\tif err := context.Background().Err(); err != nil {", "TestAContextIsMadeFromNothingOnlyWhereItIsMeantToStart", ), ( @@ -16276,4 +16276,47 @@ "map[string]any{\"Floor\": HumanBytes(core.MaxPlanBytes * 2)}", "TestTheMemorySettingIsChosenOnPreferencesAndKept", ), + # --- the manifest is written an entry at a time, the plan list reserved (2026-11-02) --- + ( + "the manifest is handed to encoding/json whole again, every entry and the document held at once", + "internal/manifest/encode.go", + "\tif len(m.Files) == 0 {\n\t\treturn encodeWhole(w, m)\n", + "\tif len(m.Files) >= 0 {\n\t\treturn encodeWhole(w, m)\n", + "TestTheManifestIsNotHeldWholeToBeWritten", + ), + ( + "a manifest that never made its list is cut as if it had one, and writes an empty list instead of null", + "internal/manifest/encode.go", + "\tif len(m.Files) == 0 {\n\t\treturn encodeWhole(w, m)\n\t}\n\thead, tail, err := m.shell()\n", + "\thead, tail, err := m.shell()\n", + "TestTheManifestIsWrittenAsEncodingJSONWritesIt", + ), + ( + "the streamed entries are joined without the line break encoding/json puts between them", + "internal/manifest/encode.go", + '\t\tbetween := []byte(",\\n")\n', + '\t\tbetween := []byte(",")\n', + "TestTheManifestIsWrittenAsEncodingJSONWritesIt", + ), + ( + "the streamed list of files closes without the indent of its level", + "internal/manifest/encode.go", + '\tif err := put(out, []byte("\\n "), tail); err != nil {\n', + '\tif err := put(out, []byte("\\n"), tail); err != nil {\n', + "TestTheManifestIsWrittenAsEncodingJSONWritesIt", + ), + ( + "the plan is grown a file at a time again instead of reserved", + "internal/engine/plantarget.go", + "\tpl.out = make([]PlannedFile, 0, max(1, pl.reserve))\n", + "\tpl.out = make([]PlannedFile, 0, 1)\n", + "TestAPlanIsReservedRatherThanGrown", + ), + ( + "the plan list is reserved before any file passed its checks, so a run refused at its first file takes the whole list first", + "internal/engine/engine.go", + "\tpl.reserve = planCapacity(targets, pl.budget)\n", + "\tpl.reserve = planCapacity(targets, pl.budget)\n\tpl.out = make([]PlannedFile, 0, pl.reserve)\n", + "TestARunRefusedAtItsFirstFileReservesNothing", + ), ] diff --git a/internal/guard/planreserve_test.go b/internal/guard/planreserve_test.go new file mode 100644 index 00000000..db70579d --- /dev/null +++ b/internal/guard/planreserve_test.go @@ -0,0 +1,65 @@ +package guard + +import ( + "runtime" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/engine" +) + +// And the reservation waits for a file that passed its checks. +// +// Reserved before the first target, a million files asking for a size their +// format cannot make were refused at the first file after taking 441-450 MB, +// against 27-30 MB before the reservation existed (review of #212, measured on +// 2026-11-02 with two runs each). In a container of half a gigabyte that is a +// refusal that can kill the process instead of being said. Asked of what Plan +// allocated, with a margin the two answers cannot cross: the reserved list is +// 440 MB, everything else here a few. Only two guards in this package run in +// parallel, so the neighbours do not reach that margin. +func TestARunRefusedAtItsFirstFileReservesNothing(t *testing.T) { + const n = 1_000_000 + targets := []engine.Target{{ID: "files", Format: "png", Sizes: engine.Uniform(n, 10)}} + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, err := engine.Plan(targets, engine.Options{OutDir: t.TempDir(), ManifestName: "manifest.json"}) + runtime.ReadMemStats(&after) + if err == nil || !strings.Contains(err.Error(), "smaller than") { + t.Fatalf("a PNG of 10 B was not refused for its size at the first file, so this is not the case it names: %v", err) + } + if took := after.TotalAlloc - before.TotalAlloc; took > 100<<20 { + t.Errorf("planning %d files refused at the first one allocated %d MB. "+ + "The list for the whole plan was reserved before any file passed its checks", n, took>>20) + } +} + +// A plan is a list reserved once, not one grown a file at a time. +// +// Grown by append, a plan of a million text files took 1.9-2.2 GB from the +// system for 0.94 GB of plan - every doubling leaves the last array to the +// collector - and reserved it took 1.2 GB with five collections instead of +// fourteen (docs/FILE-CEILING-2026-11-02.md section 2). A list that was grown +// ends with room to spare almost always, so a plan whose capacity is exactly +// its length is a plan that was reserved. +// +// What this does not hold: that the reservation stops at the memory setting. +// That half differs only in how much the process takes at its peak, and the +// heap of this test binary is shared by every guard running beside it (O162) - +// a measurement here would be about the neighbours. It is said rather than +// pretended. +func TestAPlanIsReservedRatherThanGrown(t *testing.T) { + const n = 1000 + files, err := engine.Plan([]engine.Target{{ID: "files", Format: "txt", Sizes: engine.Uniform(n, 100)}}, + engine.Options{OutDir: t.TempDir(), ManifestName: "manifest.json"}) + if err != nil { + t.Fatalf("planning %d text files: %v", n, err) + } + if len(files) != n { + t.Fatalf("the plan holds %d files, not the %d asked for, so this is not the case it names", len(files), n) + } + if cap(files) != len(files) { + t.Errorf("a plan of %d files came back with room for %d - it was grown, not reserved, "+ + "and grown it takes about twice its own size from the system", len(files), cap(files)) + } +} diff --git a/internal/manifest/encode.go b/internal/manifest/encode.go new file mode 100644 index 00000000..51130d98 --- /dev/null +++ b/internal/manifest/encode.go @@ -0,0 +1,118 @@ +package manifest + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// How a manifest becomes JSON, an entry at a time. +// +// Its own file since 2026-11-02, when writing a manifest stopped being one call +// into encoding/json and manifest.go was already past the size a person can +// follow. + +// encodeBuffer is how much of the document waits in memory before it is +// handed to the file. Large enough that a write is never a few bytes, small +// against any document worth streaming. +const encodeBuffer = 64 << 10 + +// filesList is the list of files in a document with none, as encoding/json +// writes it one level in. A key one level in sits on its own line after two +// spaces, and a string cannot hold a line break, so in a document with no +// entries this is found once and only where the list is. +var filesList = []byte("\n \"files\": []") + +// Encode renders the manifest as JSON. +// +// The bytes are the ones json.NewEncoder writes with two spaces of indent, and +// a guard holds them to that byte for byte. What changed on 2026-11-02 is when +// they exist. encoding/json builds the whole document in memory before its +// first write, and indents it into a second copy, while every entry is still +// held. Measured with the runsize probe at 100 000 text files, saving the +// manifest peaked at 9.3 KB a file against 4.0 KB for writing every file - +// about ten times the manifest's own length - and a run of 200 000 text files +// in a container of 1 GB was killed for it after its last file, leaving the +// files and no manifest to clean them up with (docs/FILE-CEILING-2026-11-02.md +// section 8). Now the document is written the way size.go counts it: the +// document with no entries, cut where its list is, and the entries in between, +// one at a time. +// +// A manifest with no entries is written whole, as it always was. It is small, +// and a list that was never made encodes as null rather than [] - written +// whole, it says exactly what it said. +func (m *Manifest) Encode(w io.Writer) error { + if len(m.Files) == 0 { + return encodeWhole(w, m) + } + head, tail, err := m.shell() + if err != nil { + return err + } + out := bufio.NewWriterSize(w, encodeBuffer) + if err := put(out, head, []byte("\n")); err != nil { + return err + } + for i := range m.Files { + // Through the slice rather than a copy, so the entry is encoded as + // the whole document encodes it: addressable, the way a slice + // element is. + entry, err := json.MarshalIndent(&m.Files[i], entryIndent, " ") + if err != nil { + return err + } + between := []byte(",\n") + if i == 0 { + between = nil + } + if err := put(out, between, []byte(entryIndent), entry); err != nil { + return err + } + } + if err := put(out, []byte("\n "), tail); err != nil { + return err + } + return out.Flush() +} + +// put writes the parts in order and stops at the first that fails. A file +// that refused a write is not asked to take the rest of a document of a +// million entries. +func put(out *bufio.Writer, parts ...[]byte) error { + for _, p := range parts { + if _, err := out.Write(p); err != nil { + return err + } + } + return nil +} + +// shell is this document with no entries, cut where its list of files is: +// head ends with the list's opening bracket and tail starts with its closing +// one. +func (m *Manifest) shell() (head, tail []byte, err error) { + empty := *m + empty.Files = []File{} + var whole bytes.Buffer + if err := encodeWhole(&whole, &empty); err != nil { + return nil, nil, err + } + doc := whole.Bytes() + if found := bytes.Count(doc, filesList); found != 1 { + return nil, nil, core.Defect(fmt.Errorf("a manifest with no entries holds %d lists of files one level in, not one", found)) + } + cut := bytes.Index(doc, filesList) + len(filesList) - len("]") + return doc[:cut], doc[cut:], nil +} + +// encodeWhole is encoding/json's rendering in one piece - the layout the +// streaming above reproduces, and the one a guard compares it with. +func encodeWhole(w io.Writer, v any) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(v) +} diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 5b263137..674f3ac0 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -522,13 +522,6 @@ func (g NoteGroup) Line(note core.Said) core.Said { return core.Says("manifest.NoteSeveral", "%s: %s Named: %s.", core.A("Files", files), core.A("Note", note), core.A("Named", named)) } -// Encode renders the manifest as JSON. -func (m *Manifest) Encode(w io.Writer) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - return enc.Encode(m) -} - // SchemaError is a manifest this build cannot read. type SchemaError struct { Path string diff --git a/internal/manifest/size.go b/internal/manifest/size.go index d23ef3b7..eca557dd 100644 --- a/internal/manifest/size.go +++ b/internal/manifest/size.go @@ -20,10 +20,15 @@ import "encoding/json" // files already peaks near three gigabytes (measured the same day), and a // second copy of the document to count it would be a large share of that. // -// The arithmetic below is Encode's layout written out: the document with an -// empty "files" list, and then the entries in place of the brackets. A guard -// holds it to the length of Encode's output, byte for byte, for documents of -// none, one and many entries (TestTheManifestSizeIsTheLengthOfTheManifest). +// The arithmetic below is the layout Encode writes (encode.go): the document +// with an empty "files" list, and then the entries in place of the brackets. +// Since 2026-11-02 Encode writes it an entry at a time rather than asking +// encoding/json for the whole document, and the two are still held apart on +// purpose: a guard holds this count to the length of Encode's output for +// documents of none, one and many entries +// (TestTheManifestSizeIsTheLengthOfTheManifest), and another holds Encode's +// bytes to encoding/json's own, so a layout both got wrong the same way is +// still caught. // entryIndent is what Encode puts in front of every line of an entry: the // entry is two levels into the document.