Skip to content

feat: push memory benchmark harness and container-writing storer - #8

Merged
Xe merged 3 commits into
mainfrom
feat/push-memory-tooling
Aug 31, 2026
Merged

feat: push memory benchmark harness and container-writing storer#8
Xe merged 3 commits into
mainfrom
feat/push-memory-tooling

Conversation

@Xe

@Xe Xe commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Two related changes from investigating how much memory a push costs.

cmd/membench

A benchmark harness for push memory. objgitd already exposed net/http/pprof
and the Go runtime collectors on the metrics listener, but nothing drove a
repeatable workload against them.

membench owns a daemon, pushes one mirror-cloned repository into many fresh
UUID-named repos, and samples /proc plus /metrics every 250ms, capturing a
heap profile whenever resident memory sets a new high-water mark. Idle
baseline, sequential phase, concurrency sweep, so per-push cost, retention
between pushes, and the slope against concurrency come apart cleanly.

See docs/usage/memory-benchmark.md.

It found the two things that shipped as #6 and #7, and confirmed both:
retained zstd state is now pinned at 64 MB where it previously varied between
64 and 432 MB run to run, and the push cap holds peak RSS at K=8 (2237 MiB)
below its own K=4 (2259 MiB), with the extra pushes appearing as wall clock
instead of memory.

internal/storage/tigris.segmentStore

This is dead code on merge. Nothing calls it and behaviour is unchanged. It
is stage 1 of 4 of docs/plans/streaming-container-sink.md, landed separately
so the risky stage lands against a reviewed foundation rather than in one large
diff.

The motivation, measured: a push materializes objects three times over, and one
push of a 48 MiB pack allocates about 4.4 GB, roughly 93x the pack.
planObjects alone is 66% of that. Live-heap profiles understate it badly
because those objects are freed as soon as their metadata is read, which is why
this went unnoticed.

segmentStore implements storer.EncodedObjectStorer over the existing
packSegment staging files, so go-git's parser can write resolved objects
straight into a .bin container instead of into a second git repository.

Reviewing

Worth knowing before stage 3:

  • The temp .pack cannot also be removed. packfile.Parser drops to its
    high-memory path when the scanner has no seeker, so parsing from the socket
    would cost more memory, not less. The plan records this.
  • Delta preservation is the open question, not memory. TestPackfileWriterKeepsDeltas
    encodes the current contract and stage 3 must not regress it.
  • Stage 2 gates on asserting byte-identical cueRecord output against the
    current implementation. A wrong raw or typ writes a repository that
    pushes fine and reads corrupt later, which no memory benchmark catches.

Happy to split this into two PRs if the dead code is unwelcome alongside the
tooling.

Testing

go build ./..., go vet ./..., go test ./... all clean. 9 new test
functions for segmentStore, 11 for membench.

Xe added 3 commits August 31, 2026 16:29
objgitd already served net/http/pprof and the Go runtime collectors on the
metrics listener, but nothing drove a repeatable push workload against them,
so per-push memory cost was guesswork.

membench owns a daemon, pushes one mirror-cloned repository into many fresh
UUID-named repos, and samples /proc plus /metrics every 250ms, capturing a
heap profile whenever resident memory sets a new high-water mark. It runs an
idle baseline, a sequential phase, and a concurrency sweep, so per-push cost,
retention between pushes, and the slope against concurrency are separable.

Two measurement details are load-bearing. Peaks come from VmHWM rather than
sampled VmRSS, because the kernel's own high-water mark cannot be missed
between ticks. Peak captures use gc=0, since forcing a collection would move
the number that triggered the capture.

A failed /metrics scrape keeps its /proc numbers but records no runtime
figures: the last tick before shutdown routinely fails, and treating its zero
values as a measurement reported a settled heap of zero.

Repositories are never deleted. The run writes repos.txt and leaves cleanup to
the operator.

Assisted-by: Claude Opus 5 via Claude Code
Claude-Session: https://claude.ai/code/session_01PmugaKamcmXCxM4u4w29iD
Signed-off-by: Xe Iaso <me@xeiaso.net>
First stage of replacing the scratch go-git repository that PackfileWriter
decodes a push into. Not yet wired to the write path: nothing calls
segmentStore, and behaviour is unchanged.

A push currently materializes objects three times over. Parser.Parse resolves
every delta into scratch, planObjects resolves every delta again to read hash,
type and size, and payloadFor materializes a third time through deltaForm to
pick a delta form. Measured with cmd/membench, one push of a 48 MiB pack
allocates about 4.4 GB, roughly 93x the pack, and planObjects alone accounts
for 66% of it. Live-heap profiles understate this badly, because those objects
are freed as soon as their metadata is read.

segmentStore implements storer.EncodedObjectStorer over the existing
packSegment staging files, so go-git's parser can write each resolved object
straight into a .bin container instead of into a second git repository.
RawObjectWriter stages an object to its own temp file while hashing, then
hands it to packSegment.add, which keeps every container format decision in
one place. Staging files stay open for the whole push because the parser reads
bases back out, and a base can live in a container that filled long before the
delta needing it arrives. An index miss falls through to the backing Storer,
which is the thin-pack path.

The plan document records why the temp .pack cannot also be removed:
packfile.Parser drops to its high-memory path when the scanner has no seeker,
so parsing straight from the socket would cost more memory, not less.

Refs: docs/plans/streaming-container-sink.md
Assisted-by: Claude Opus 5 via Claude Code
Claude-Session: https://claude.ai/code/session_01PmugaKamcmXCxM4u4w29iD
Signed-off-by: Xe Iaso <me@xeiaso.net>
staticcheck S1021. The split declaration was left over from an earlier
version of the test that assigned the hash conditionally.

Assisted-by: Claude Opus 5 via Claude Code
Claude-Session: https://claude.ai/code/session_01PmugaKamcmXCxM4u4w29iD
Signed-off-by: Xe Iaso <me@xeiaso.net>
@Xe
Xe merged commit f322c9f into main Aug 31, 2026
6 checks passed
Xe pushed a commit that referenced this pull request Aug 31, 2026
# [1.5.0](v1.4.1...v1.5.0) (2026-08-31)

### Features

* push memory benchmark harness and container-writing storer ([#8](#8)) ([f322c9f](f322c9f)), closes [hi#water](https://github.com/hi/issues/water) [hi#water](https://github.com/hi/issues/water) [hi#memory](https://github.com/hi/issues/memory)

Signed-off-by: Tigris Data <social@tigrisdata.com>
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant