From f6997b16c4e4cbfb98c651bd69c301d7bec46c14 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 6 Sep 2026 04:42:48 +0200 Subject: [PATCH] perf: an archive names its directories once and encrypts into one buffer Two findings from the performance report, both in internal/format/archive and neither moving a byte. P9: Layout.Path rebuilt the directory chain for every entry, through a fmt format verb, though the chain depends only on the depth. Prefix() builds it and the two hot callers hoist it out of their loops. Measured at depth 50 over 10 000 entries: 82.2 ms before, 30.6 ms once the verb went, and close to nothing with the prefix built once. The default depth is zero, where the prefix is empty and none of this was ever paid - so this is the ceiling of a setting rather than a run anybody has. P10: the two locked-entry writers allocated a fresh buffer on every Write. At 128 MB in 32 kB blocks that is about four thousand allocations. The buffer now lives as long as the entry. It cannot be done in place: p belongs to the caller and the zip writer passes the same slice on, so scrambling it would corrupt what somebody else is about to read. That is written next to both writers. The report never measured P10 - entryWriter is unexported - and the clock does not settle it either, since the processor time ranges overlap. The collector count does, and it is deterministic: a 128 MB locked zip went from 48 collections to 6 with aes-256 and 7 with zipcrypto. No new guard, deliberately. Path length against LongestPath and the locked archive's exact size were both already guarded, and adding a fourth defence beside three is the shape this project has thrown away seven times. Three mutations prove those guards catch the broken version. One mutation pattern went stale in the same step, quoting the call this change rewrote, and staleness.py caught it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 +++++ internal/format/archive/layout.go | 60 ++++++++++++++++++++-------- internal/format/archive/lock.go | 11 ++++- internal/format/archive/zipcrypto.go | 9 ++++- internal/format/targz/targz.go | 5 ++- internal/format/zip/children.go | 6 ++- 6 files changed, 81 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d83db14..71db9ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,17 @@ because it turns other people's test suites red. in front of the picture, so it has to know how large the picture is before it starts. +- **A password protected archive allocates once per entry instead of once per + block written.** Producing a 128 MB locked `.zip` used to make the collector + run 48 times. It runs 6. The files are identical and the wall clock barely + moves, so this is headroom rather than a speed-up you will notice. + +- **Nesting files deep inside an archive costs almost nothing to name.** The + directory chain in front of every entry was rebuilt for each one, though it + depends only on the depth. At the deepest setting with 10 000 entries that was + 82 ms of naming, and it is now close to nothing. Archives left flat, which is + the default, never paid it either way. + - **`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/archive/layout.go b/internal/format/archive/layout.go index c2023cc..4c0a67b 100644 --- a/internal/format/archive/layout.go +++ b/internal/format/archive/layout.go @@ -1,7 +1,6 @@ package archive import ( - "fmt" "strconv" "strings" @@ -59,18 +58,22 @@ const ( // format rather than trusting this paragraph. maxDepth = 50 - // dirSegment numbers the levels so a path reads as what it is. Two digits - // because maxDepth is two digits, and a fixed width so every segment is - // the same size and the arithmetic above stays a multiplication. - dirSegment = "d%02d/" - - // dirSegmentBytes is what one segment comes to once rendered - "d00/" is - // four bytes where the format string above is six. Written out rather than - // taken as len(dirSegment), which is the bug the depth guard caught the - // first time it ran: the arithmetic said every path was 2 bytes per level - // longer than it is, which would have understated the ceiling rather than - // overstating it, so nothing would have failed until somebody widened the - // segment. A guard compares this against a really rendered path. + // A level is written as "d00/": the letter, the number padded to two + // digits because maxDepth is two digits, and the separator. A fixed width + // is what keeps the arithmetic above a multiplication rather than a walk. + // + // It used to be a "d%02d/" format string rendered through fmt, and that + // cost more than everything else about naming an entry put together - + // measured 2026-09-06 at depth 50 over 10 000 entries, 82.2 ms against + // 30.6 ms once the verb went. Prefix writes the four bytes out by hand. + // + // dirSegmentBytes is what one level comes to. It was written out rather + // than taken as the length of that format string, which is the bug the + // depth guard caught the first time it ran: six bytes rather than four + // said every path was 2 bytes per level longer than it is, which + // understates the ceiling instead of overstating it, so nothing would have + // failed until somebody widened the segment. A guard still compares this + // against a really rendered path. dirSegmentBytes = 4 ) @@ -87,15 +90,38 @@ type Layout struct { // The empty name gives the directory chain itself with its trailing slash, // which is what both containers want a directory entry to be called. func (l Layout) Path(name string) string { + return l.Prefix() + name +} + +// Prefix is the directory chain on its own, with nothing on the end. +// +// It depends on Depth and on nothing else, so a caller naming thousands of +// entries works it out once rather than once per entry. Measured 2026-09-06 at +// depth 50 over 10 000 entries: a median of 82.2 ms rebuilding it every time +// against 1.08 ms building it once, ranges disjoint. +// +// The default depth is zero and a flat archive spends nothing here either way, +// so this is a ceiling rather than a typical run - which is worth saying, +// because the number above reads like a saving every user gets. +// +// Rendered by hand rather than through fmt: the format verb is what made the +// old version expensive, and a two digit number with a floor of two is small +// enough to write out. "d%02d/" pads to two and lets a third digit through, +// which is what the branch below does. +func (l Layout) Prefix() string { if l.Depth <= 0 { - return name + return "" } var b strings.Builder - b.Grow(l.Depth*len(dirSegment) + len(name)) + b.Grow(l.Depth * dirSegmentBytes) for i := 0; i < l.Depth; i++ { - fmt.Fprintf(&b, dirSegment, i) + b.WriteByte('d') + if i < 10 { + b.WriteByte('0') + } + b.WriteString(strconv.Itoa(i)) + b.WriteByte('/') } - b.WriteString(name) return b.String() } diff --git a/internal/format/archive/lock.go b/internal/format/archive/lock.go index a34d074..3552f2f 100644 --- a/internal/format/archive/lock.go +++ b/internal/format/archive/lock.go @@ -274,6 +274,8 @@ type entryWriter struct { out io.Writer ctr *counter mac hash + // buf is reused across writes. See Write. + buf []byte } // hash is the part of hash.Hash this uses. Named so the field above reads as @@ -284,7 +286,14 @@ type hash interface { } func (e *entryWriter) Write(p []byte) (int, error) { - out := make([]byte, len(p)) + // A scratch buffer that lives as long as the entry rather than one per + // call. It cannot be done in place: p belongs to the caller, and the zip + // writer hands the same slice on elsewhere, so scrambling it here would + // corrupt what somebody else is about to read. + if cap(e.buf) < len(p) { + e.buf = make([]byte, len(p)) + } + out := e.buf[:len(p)] e.ctr.xor(out, p) if _, err := e.mac.Write(out); err != nil { return 0, err diff --git a/internal/format/archive/zipcrypto.go b/internal/format/archive/zipcrypto.go index e9684af..b20c5e5 100644 --- a/internal/format/archive/zipcrypto.go +++ b/internal/format/archive/zipcrypto.go @@ -82,6 +82,8 @@ func (c *pkware) encrypt(p byte) byte { type zipCryptoWriter struct { out io.Writer c *pkware + // buf is reused across writes. See Write. + buf []byte } // newZipCryptoWriter starts an entry, writing the twelve byte header before @@ -111,7 +113,12 @@ func (l Lock) newZipCryptoWriter(w io.Writer, seed uint64, index int, crc uint32 } func (z *zipCryptoWriter) Write(p []byte) (int, error) { - out := make([]byte, len(p)) + // Reused across writes, and not done in place for the reason written out + // on entryWriter.Write: p belongs to the caller. + if cap(z.buf) < len(p) { + z.buf = make([]byte, len(p)) + } + out := z.buf[:len(p)] for i := range p { out[i] = z.c.encrypt(p[i]) } diff --git a/internal/format/targz/targz.go b/internal/format/targz/targz.go index 6c07861..554e29d 100644 --- a/internal/format/targz/targz.go +++ b/internal/format/targz/targz.go @@ -252,6 +252,9 @@ func (generator) Plan(r format.Request) (format.Plan, error) { // seed of a member does not move when a group above it changes count. That is // untouchable rule 2 applied one level down. func planChildren(r format.Request, groups []format.Content, layout archive.Layout) ([]child, error) { + // The directory chain depends only on the depth, so it is built once here + // rather than once per entry. + prefix := layout.Prefix() var out []child index := 0 // Numbering runs per format rather than per group, so two groups of the @@ -273,7 +276,7 @@ func planChildren(r format.Request, groups []format.Content, layout archive.Layo } numbered[g.Format]++ out = append(out, child{ - name: layout.Path(fmt.Sprintf("%s_%04d%s", g.Format, numbered[g.Format], desc.Extension)), + name: prefix + fmt.Sprintf("%s_%04d%s", g.Format, numbered[g.Format], desc.Extension), desc: desc, plan: cp, }) diff --git a/internal/format/zip/children.go b/internal/format/zip/children.go index 466e155..3e3d2bf 100644 --- a/internal/format/zip/children.go +++ b/internal/format/zip/children.go @@ -24,6 +24,10 @@ import ( // seed of a member does not move when a group above it changes count. That is // untouchable rule 2 applied one level down. func planChildren(r format.Request, groups []format.Content, layout archive.Layout) ([]child, error) { + // The directory chain depends only on the depth, so it is built once here + // rather than once per entry. + prefix := layout.Prefix() + // Sized up front, because the total is known before the walk starts: it is // what the groups add up to. Growing by append instead reallocates and // copies the whole slice fourteen times on the way to ten thousand entries, @@ -54,7 +58,7 @@ func planChildren(r format.Request, groups []format.Content, layout archive.Layo } numbered[g.Format]++ out = append(out, child{ - name: layout.Path(fmt.Sprintf("%s_%04d%s", g.Format, numbered[g.Format], desc.Extension)), + name: prefix + fmt.Sprintf("%s_%04d%s", g.Format, numbered[g.Format], desc.Extension), desc: desc, plan: cp, })