Skip to content

Commit 2bd51ec

Browse files
authored
perf(storage/tigris): push any repository in bounded memory (#25)
* perf(storage/tigris): index pushed packs in bounded memory A push decoded its pack through a scratch go-git filesystem.Storage. That storage parses with no storage attached, so go-git's parser stays out of low-memory mode and keeps every inflated base and resolved delta until the parse ends. A push of golang/go (465 MiB pack) passed 21 GiB of heap and did not complete. Replace it with a small index-pack in indexpack.go. One pass stages, checksums, and indexes the pack as it arrives, with no bodies kept. A second pass resolves each delta tree depth first, so a body lives only while a delta below it needs it. Bodies over 8 MiB, or past a 64 MiB total, go to temp files. writePack hands the stream over through a new optional ReadPack method, which also removes a go-git Scanner that buffered every object body in full. golang/go now peaks at about 521 MiB of heap and takes 23 s. Xe/x goes from 516 MiB and 14.7 s to about 165 MiB and 1.3 s. Delta reuse is unchanged. This also fixes stored delta chains of exactly 50 links, which git makes by default and which reads refused. Reads now accept a chain of maxDeltaDepth links, and the writer stores an object whole rather than extend a stored chain past that, since clients can send 4095. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * docs: plan bounded push memory and describe the new pack writer Record the measurements behind the new pack writer, the delta chain depth fault it found, and the next phase: a compact pack index to replace the per-object entries map. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * perf(storage/tigris): replace the pack index map with sorted tables The pack index kept a map[plumbing.Hash]packEntry, about 210 bytes for each object. Every push held one entry for each object it wrote, and every clone or fetch built one for each object in the repository. Keep the entries in immutable tables instead: the hashes in one sorted []byte and a 40-byte record for each object, about 60 bytes with SHA-1. A record names its pack by number and its delta base by record number. Each registered container adds a table, and more than 16 tables merge into one, so a lookup is at most 16 binary searches. deregister takes only a pack id, so packJob no longer holds its cueRecords until the upload ends. The iterator walks 8-byte record references instead of a copy of every entry, and yields a hash only from the record the lookup returns, which removes its set of seen hashes. A snapshot shares the tables with the index rather than copying them, because the index never edits them in place. For golang/go, a cold index build drops from 225 MiB to 113 MiB of live heap (the index itself from about 180 MiB to 40 MiB). A push peaks at 364 MiB instead of 521 MiB. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * perf(storage/tigris): cut system calls on the push path A CPU profile of a golang/go push spent about 80% of its time in system calls. packSegment wrote each object to its file with no buffer, which cost 14.7 s of a 26 s push. Write through a 1 MiB buffer and count the bytes, so copyCompressed no longer seeks to measure a payload. The unused segmentStore reads from an open segment, so it flushes first. Each inflate also read its section through a 32 KiB buffer, so even a tiny object read 32 KiB. Read a section of 64 KiB or less whole, with one ReadAt of its own length. Wall time did not move measurably on a laptop: one pread per inflate now dominates. The plan records mmap of the staged pack as the next step. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * docs: record the compact pack index and its measurements Describe the table-based pack index in the storer architecture, and add the phase 2 results and the remaining push-time work to the plan. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * style(storage/tigris): gofmt iter.go Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * test(storage/tigris): cover the pack reader with hand-built packs The existing pack tests reach the new pack reader only through Write, with packs from the git CLI, so they never send a REF-delta, a delta whose base is already in the repository, a chain deeper than a read walks, or a malformed pack. Build packs by hand and push each one through both ReadPack and Write. The round-trip cases read every object back through a new Storer and check the stored delta count and the longest stored chain. The reject cases check the error and that no PutObject call was made. One more test pins that ReadPack leaves the bytes after the trailer unread, which writePack depends on for git:// and SSH. Removing the chain-depth cap or the delta source-size check makes these tests fail. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> * docs: format the push memory plan with prettier Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com> --------- Signed-off-by: Xe Iaso <xe@tigrisdata.com>
1 parent f4b0030 commit 2bd51ec

14 files changed

Lines changed: 2589 additions & 430 deletions

File tree

‎cmd/objgitd/receivepack.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,17 @@ func writePack(st storage.Storer, rd io.Reader) error {
268268
return err
269269
}
270270

271+
// A writer that reads the pack itself finds its end in the same pass that
272+
// stores it. The Scanner below cannot do that in bounded memory: over a
273+
// reader that cannot seek, it buffers every object's whole inflated body.
274+
if pr, ok := w.(packReader); ok {
275+
if err := pr.ReadPack(rd); err != nil {
276+
_ = w.Close()
277+
return err
278+
}
279+
return w.Close()
280+
}
281+
271282
sc := packfile.NewScanner(io.TeeReader(rd, w), sopts...)
272283
for sc.Scan() {
273284
}
@@ -278,6 +289,12 @@ func writePack(st storage.Storer, rd io.Reader) error {
278289
return w.Close()
279290
}
280291

292+
// packReader is the optional PackfileWriter surface that reads exactly one
293+
// packfile from r and stops at its trailer. The tigris storer has one.
294+
type packReader interface {
295+
ReadPack(r io.Reader) error
296+
}
297+
281298
// sidebandProgress writes to the sideband progress channel (band 2), which the
282299
// git client renders as "remote: " lines.
283300
type sidebandProgress struct {

‎docs/architecture/tigris-storer.md‎

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -339,26 +339,47 @@ caps below set how much one container holds.
339339
It does not store the git pack format. Git packs use delta compression, so one
340340
read would have to resolve a chain of deltas.
341341

342-
`PackfileWriter` instead writes the incoming pack to a scratch
343-
`storage/filesystem.Storage` on a local temp directory. That storage decodes
344-
the pack for us, so this package holds no pack-parsing code.
345-
346-
The writer makes two passes. The first reads the hash, type, size, and delta
347-
base of every object, then orders them so a base always precedes the delta
348-
that needs it — `IterEncodedObjects` returns hash order, which gives no such
349-
guarantee. The second pass copies each payload into a flat `packs/<id>.bin`
350-
and adds one record to `packs/<id>.cue`. A record holds the hash, the type,
351-
the payload codec, the offset, the stored length, the raw size, and the base
352-
hash. `<id>` is the hex SHA-256 of the `.bin`, so the name doubles as a
353-
checksum. Both files upload through the same uploader as loose objects, so the
354-
existing `SetReference` flush covers them.
342+
`PackfileWriter` decodes the pack itself, in `indexpack.go`. That file is a
343+
small version of `git index-pack`. Its memory does not grow with the size of
344+
the history or with the size of one object. See
345+
[the plan](../plans/push-memory.md) for the measurements.
346+
347+
The writer makes two passes:
348+
349+
1. `scanPack` reads the pack as it arrives. It stages the bytes in a temp
350+
file, verifies the trailer checksum, and keeps a small index entry for each
351+
object: the offset, the size, the type, and the base of a delta. It keeps
352+
no body.
353+
2. The resolver walks each delta tree depth first, from its base, so a base
354+
always goes into a container before its deltas. It copies each payload into
355+
a flat `packs/<id>.bin` and adds one record to `packs/<id>.cue`.
356+
357+
A record holds the hash, the type, the payload codec, the offset, the stored
358+
length, the raw size, and the base hash. `<id>` is the hex SHA-256 of the
359+
`.bin`, so the name is also a checksum. Both files upload through the same
360+
uploader as loose objects, so the existing `SetReference` flush covers them.
361+
362+
A body stays in memory only while a delta below it needs it as a base. A
363+
body goes to a temp file when it is larger than `bodyMemLimit` (8 MiB), or when
364+
the bodies in memory would pass `bodyMemBudget` (64 MiB). The delta applier
365+
reads its base through `io.ReaderAt`, so a base on disk is never loaded whole.
366+
367+
`cmd/objgitd`'s `writePack` calls `ReadPack` when the writer has it. `ReadPack`
368+
finds the end of the pack in the same pass that stages it. `packScanner` is an
369+
`io.ByteReader`, so zlib never reads past the pack. On git:// and SSH the
370+
client keeps the connection open after the pack, so a read past the pack
371+
blocks.
355372

356373
A payload is the object, or the delta the pushing client already computed for
357374
it. Keeping that delta is what stops every later clone from deriving one
358375
again: `Storer` implements `storer.DeltaObjectStorer`, so go-git's packer
359376
reuses the stored delta instead of running its rolling-hash search. See
360377
[the backend reference](../reference/tigris-backend.md#how-a-read-rebuilds-a-delta).
361378

379+
A read walks at most `maxDeltaDepth` (50) delta links, and a client can send
380+
chains of up to 4095 links. The writer therefore stores an object whole when
381+
its stored chain is already 50 links long.
382+
362383
A delta is only kept when its base lands in the same container. When a
363384
container seals between the two, the writer stores the whole object instead.
364385
That costs a few deltas at each boundary and buys back the rule that every
@@ -417,6 +438,41 @@ Reads check three tiers, in this order:
417438
The pack index is built once for each `Storer`, from the `packs/*.cue` files.
418439
Those files are small, and no `.bin` body is read to build it.
419440

441+
### The pack index (`packtable.go`)
442+
443+
The index is a list of sorted tables, and not a map. A table holds the hashes
444+
of its records in one `[]byte`, and a 40-byte `tableRec` for each record. That
445+
is about 60 bytes for each object with SHA-1. The map it replaced took about
446+
210 bytes. For golang/go, a cold build holds 40 MiB of tables, against about
447+
180 MiB of map.
448+
449+
A `tableRec` holds a pack number and not a pack id, and the base of a delta as
450+
a record number and not a hash. A base in another table goes in the small
451+
`farBase` map. The writer keeps a base and its delta in one container, so that
452+
map is almost always empty.
453+
454+
The cold build parses each `.cue` into a table and then drops its records. It
455+
then merges all of the tables into one. Each container that a push registers
456+
adds one table. When there are more than `maxPackTables` (16) tables, the index
457+
merges them into one, so a lookup does at most 16 binary searches. A lookup
458+
searches from the newest table to the oldest, so the newest record of a hash
459+
wins.
460+
461+
A table never changes after it is built. A `packSnapshot` therefore shares the
462+
table list, the pack ids, and the dead set with the index, and costs no copy.
463+
The index never edits these in place. It only appends past the length that a
464+
snapshot sees, or it replaces the slice or the map.
465+
466+
`deregister` takes only a pack id. It drops a table that holds only that pack.
467+
In a merged table it adds the pack to the dead set, a lookup skips the record,
468+
and the next merge drops it. `packJob` therefore keeps no `cueRecord`s while
469+
its upload runs.
470+
471+
`IterEncodedObjects` walks `packSnapshot.order`, which costs 8 bytes for each
472+
object, in pack order and then offset order. It yields a hash only from the
473+
record that the lookup returns, so it needs no set of hashes that it has
474+
already yielded.
475+
420476
A packed read is one ranged `GetObject` over the record's `stored` span. There
421477
is no delta chain to reconstruct. When the record names the zstd codec, the
422478
read decompresses one frame; otherwise the bytes are already the object.

‎docs/plans/push-memory.md‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
# Plan: bounded memory for large pushes
2+
3+
## Goal
4+
5+
A push of any repository, of any size, must complete in bounded memory. The
6+
memory that one push uses must not grow with the size of the history, and must
7+
not grow with the size of one object.
8+
9+
## The problem
10+
11+
A push of golang/go (465 MiB pack, 708,000 objects) used more than 21 GiB of
12+
heap. On a laptop it filled swap and did not complete. A push of Xe/x (50 MB
13+
pack, 21,000 objects) used 516 MiB.
14+
15+
Almost all of that memory was in go-git, and not in this repository.
16+
17+
`PackfileWriter` gave the pushed pack to a scratch go-git
18+
`filesystem.Storage`. That storage indexes the pack with a `packfile.Parser`
19+
that has no storage attached. Without a storage, the parser does not use its
20+
low-memory mode. It keeps the inflated body of every base object and every
21+
resolved delta until the whole pack is parsed. Memory therefore grew with the
22+
uncompressed size of the full history.
23+
24+
Three smaller costs came after the parse:
25+
26+
- The plan pass called `IterEncodedObjects`, which loads each full object to
27+
read its hash, type, and size.
28+
- Eight workers opened eight more copies of the scratch index to find delta
29+
bases.
30+
- The container walk resolved each object again through the go-git packfile
31+
reader.
32+
33+
A fourth cost was in `cmd/objgitd/receivepack.go`. `writePack` ran a go-git
34+
`Scanner` over the socket to find the end of the pack. That reader cannot
35+
seek, so the scanner buffered the full inflated body of each object. One large
36+
blob therefore needed its full size in memory.
37+
38+
## Phase 1: replace the scratch storage (done)
39+
40+
`internal/storage/tigris/indexpack.go` is a small version of `git index-pack`.
41+
It does the work in two passes.
42+
43+
1. `scanPack` reads the pack once, as it arrives. It stages the bytes in a
44+
temp file and verifies the trailer checksum. For each object it keeps about
45+
60 bytes: the offset, the size, the type, and the base of a delta. It
46+
inflates each entry to find where the entry ends, but it keeps no body.
47+
2. The resolver walks each delta tree depth first, from its base. A body lives
48+
only while a delta below it needs it as a base. A linear chain therefore
49+
holds two bodies at a time. Each object is inflated, resolved, and hashed
50+
once.
51+
52+
A body stays in memory when it is 8 MiB or less, and when all bodies in memory
53+
stay under 64 MiB. Other bodies go to a temp directory for the push. The delta
54+
applier reads its base through `io.ReaderAt`, so it never loads a base on disk
55+
into memory.
56+
57+
`writePack` now hands the stream to the storer through an optional
58+
`ReadPack(io.Reader) error` method. The storer finds the end of the pack in the
59+
same pass that stages and indexes it. `packScanner` is an `io.ByteReader`, so
60+
zlib does not read past the end of the pack. This matters on git:// and SSH,
61+
where the client keeps the connection open after the pack.
62+
63+
### Results of phase 1
64+
65+
The numbers come from a local harness. It feeds a real pack to
66+
`PackfileWriter` over an S3 fake that discards bodies.
67+
68+
| Push | Before | After |
69+
| ------------------------- | --------------------- | ---------------- |
70+
| golang/go, peak heap | more than 21 GiB | 521 MiB |
71+
| golang/go, time | did not complete | 23 s |
72+
| Xe/x, peak heap | 516 MiB | about 165 MiB |
73+
| Xe/x, time | 14.7 s | 1.3 s |
74+
| 266 MB blob and one delta | both bodies in memory | 35 MiB peak heap |
75+
76+
A second test reads every object back through a new `Storer` and verifies
77+
its hash. It passes for all 708,000 objects of golang/go. It also passes for a
78+
chain of 50 links, a chain of 197 links, and the 266 MB blob. Each case passes
79+
with the default memory bound, and again with every body sent to disk.
80+
81+
The two writers store the same deltas. For Xe/x, both keep 12,016 deltas and
82+
both store 39 MiB.
83+
84+
### Spill to disk by default?
85+
86+
A run that sends every body to disk took 140 s, against 23 s. System time went
87+
from 5 s to 109 s. Most objects are a few kilobytes, so each one costs file
88+
system calls that are larger than the work. Each copy command in a delta also
89+
becomes one `pread`. The peak heap fell by only 33 MiB, because the 64 MiB
90+
budget already bounds the bodies in memory.
91+
92+
## A bug that phase 1 found
93+
94+
A stored delta chain of exactly 50 links did not read back. `deltaBase`
95+
refused depth 50, but depth 50 is the link that reaches the whole object. Git
96+
uses a default `pack.depth` of 50, so normal pushes make such chains. The
97+
writer on `main` has the same fault. The memory problem hid it, because
98+
golang/go never got far enough.
99+
100+
A client can also send chains of up to 4095 links, and a read walks only 50.
101+
Two changes correct this:
102+
103+
- A read now accepts a chain of exactly `maxDeltaDepth` links.
104+
- If a stored chain is already `maxDeltaDepth` links long, the writer stores
105+
the next object whole. In a chain of 197 links, this stores 10 of 803 objects
106+
whole.
107+
108+
## Phase 2: make the pack index compact (done)
109+
110+
After phase 1, the memory that remains grows with the number of objects, at
111+
about 390 bytes for each live object. For golang/go that is about 275 MiB.
112+
For a repository of 10 million objects, it is about 4 GB.
113+
114+
| Part | Bytes per object | golang/go |
115+
| ------------------------------ | ---------------: | --------: |
116+
| `packIndex.entries` map | about 210 | 149 MiB |
117+
| `cueRecord` slices of the push | about 70 | 48 MiB |
118+
| push scan index | about 50 | 35 MiB |
119+
| zstd history (fixed) | – | 64 MiB |
120+
121+
The map is the largest part. Reads use it too: a clone or a fetch builds the
122+
same map for every object in the repository. A smaller map therefore helps
123+
both the push and the read path.
124+
125+
### Design
126+
127+
Replace `entries map[plumbing.Hash]packEntry` with a list of sorted tables.
128+
129+
- A table holds the hashes of its objects as one `[]byte`, sorted, with
130+
`hashSize` bytes for each hash.
131+
- A parallel slice holds a small fixed-size record for each object: the pack
132+
number, the offset, the stored length, the raw size, the type, and the
133+
codec.
134+
- The base of a delta is an index into the same table. A base in another
135+
table goes in a small side map. The writer keeps a base and its delta in the
136+
same container, so the side map is almost always empty.
137+
- A table has a slice of pack ids. Each record holds a `uint32` index into
138+
that slice, and not a `string`.
139+
140+
This takes about 50 bytes for each object, against about 210 bytes now.
141+
142+
`packLookup` does a binary search in each table. The cold build makes one
143+
table from every `.cue`. Each container that a push registers adds one table.
144+
When there are more than 16 tables, the index merges them into one table, so
145+
a lookup never searches more than 16 tables.
146+
147+
`deregister` removes a table that is still alone. For a table that a merge
148+
absorbed, it adds the pack id to a set of dead packs, and a lookup skips a
149+
record from a dead pack. `deregister` then needs only the pack id, so
150+
`packJob` no longer holds its `[]cueRecord` until the upload completes.
151+
152+
`snapshotEntries` keeps its order, by pack and then by offset. It reads that
153+
order from the tables.
154+
155+
`packEntry` stays as the type that `packLookup` returns. The lookup builds it
156+
from a record, so callers do not change.
157+
158+
### Order of work
159+
160+
1. Add the table type with lookup, merge, and dead packs. Give it
161+
table-driven tests.
162+
2. Move `indexRecords`, `register`, `deregister`, `packLookup`, and
163+
`snapshotEntries` to the tables.
164+
3. Change `packJob` and `deregister` to use the pack id only.
165+
4. Make the cold build parse each `.cue` into a table, and then release the
166+
parsed records.
167+
5. Measure the push and a cold index build of golang/go again.
168+
169+
### Results of phase 2
170+
171+
The tables work as the design says, with two changes:
172+
173+
- The iterator does not copy the entries. It keeps an 8-byte reference to each
174+
record, and yields a hash only from the record that the lookup returns. This
175+
removes the `seen` map, which cost more than 48 bytes for each object.
176+
- A snapshot shares the table list, the ids, and the dead set with the index,
177+
and does not copy them. A lookup takes a snapshot, so a copy there cost a
178+
clone of every pack id on every lookup.
179+
180+
| Measurement (golang/go) | Phase 1 | Phase 2 |
181+
| -------------------------------- | ------: | ------: |
182+
| Cold index build, live heap | 225 MiB | 113 MiB |
183+
| Of which, the index itself | 180 MiB | 40 MiB |
184+
| Push, peak heap | 521 MiB | 364 MiB |
185+
| Push, live heap after the upload | 274 MiB | 114 MiB |
186+
187+
The other 73 MiB of the cold build is fixed. 45 MiB is the buffers of the
188+
`.cue` zstd decoder, and 28 MiB is the bodies that the S3 fake keeps.
189+
190+
The round-trip test passes for all 708,000 objects of golang/go, and for every
191+
other case of phase 1. The race detector finds no race in the package.
192+
193+
### Two changes for push time
194+
195+
A CPU profile of the push showed that system calls took about 80% of the
196+
time. Two causes were in this package:
197+
198+
- `packSegment` wrote to its file with no buffer. Each object cost at least one
199+
`write` call, which was 14.7 s of a 26 s push. The segment now writes
200+
through a 1 MiB buffer and counts its bytes, so `copyCompressed` needs no
201+
`Seek`.
202+
- Each inflate read its section through a 32 KiB buffer, so a small object
203+
still read 32 KiB. A section of 64 KiB or less is now read with one `ReadAt`
204+
of its own length.
205+
206+
The wall time did not change by a measurable amount, at 25 s to 28 s on a
207+
laptop. One `pread` for each inflate, about 1.4 million calls, now takes most
208+
of the time. See phase 3.
209+
210+
## Phase 3: work to examine later
211+
212+
- **Index size for very large repositories.** After phase 2, about 110 bytes
213+
for each object remain during a push: the scan index and the tables. For 10 million objects that is about
214+
1.1 GB. A table can live in a memory-mapped temp file, and then the kernel
215+
can page it out.
216+
- **One `pread` for each inflate.** A memory map of the staged pack removes
217+
these calls. The mapped pages count in the RSS of the process, but they are
218+
file pages that the kernel can drop. A `pread` puts the same pages in the
219+
page cache, and cgroup accounting counts them in both cases.
220+
- **The `.cue` zstd decoder.** It keeps about 45 MiB of buffers after its
221+
first use. A lower decoder concurrency cuts that.
222+
- **The zstd encoder.** It keeps 64 MiB of history for each process.
223+
`EncodeAll` inputs are at most `inMemoryCap` (1 MiB), except `.cue` blocks.
224+
A 1 MiB window cuts that to about 8 MiB. This changes the compressed bytes,
225+
so it needs its own measurement.
226+
- **Large bases in their own container.** An object larger than
227+
`maxPackBytes` gets a container of its own. A delta against it lands in the
228+
next container, so the writer stores that delta whole. This behavior is
229+
older than this plan.

0 commit comments

Comments
 (0)