fix(nwsync): stream emit so peak memory tracks the largest resource (#76) (#78)
build-binaries / build-binaries (push) Successful in 2m13s
build-binaries / build-binaries (push) Successful in 2m13s
Closes #76. Part of #54. `nwsync emit` held roughly 5× the artifact size in RAM, so ovh-main (7 GB, no swap) OOM-killed it on any hak over ~1.4 GB. That blocked the backfill in sow-assets-manifest and would have killed the next release rebuilding a large hak. ## What it does - `erf.ReadIndex` / `erf.ReadPayload` — parse the header and resource table only, read one payload on demand. `erf.Read` keeps its shape but returns payloads as subslices of the buffer instead of fresh copies, which removes one full copy for the `pipeline` callers too. Callers must not mutate `Resource.Data`; the doc comment says so and no caller does. - `nwsync.Emit` opens the artifact, hashes it by streaming for the key check (through a section reader, so the file offset stays put), then hashes, compresses and stores one resource at a time. The archive is never resident. Shadowed duplicate resrefs are now never read at all. - Bounds checks in `ReadIndex` moved to `int64`, so key/resource-list offsets can no longer overflow. ## Not in the issue, but memory-motivated The zstd blob encoder ran at the default concurrency, which is one encoder per CPU, each holding a window-sized history — about 200 MB of live heap doing nothing on a 24-core runner. `EncodeAll` is single-threaded per call, so concurrency 1 costs nothing. `TestSingleThreadedEncoderMatchesDefault` pins the claim that blobs come out byte-identical. ## Measured Peak heap during emit, sampled 1 ms: | hak | before | after | |-----|--------|-------| | 8 MB | 155 MB | 21 MB | | 64 MB | 289 MB | 22 MB | Flat, as the acceptance asks. `TestEmitPeakMemoryDoesNotScaleWithArtifactSize` fails if the 64 MB fixture costs more than the 8 MB one plus 24 MB of slack. ## Gaps - The regression check measures Go heap, not RSS, and its largest fixture is 64 MB — a multi-GB run was not done here. A 2.15 GB hak now needs about the same ~22 MB the 64 MB one does, so the 5 GB budget is not close, but that is inference from the flat curve, not a measurement. - mmap was suggested in the issue and skipped. Payload buffers are still anonymous, but they are one resource each (≤15 MiB), so making them file-backed buys nothing now. 🤖 Generated with [Claude Code](https://claude.com/claude-code)Reviewed-on: #78 Reviewed-by: xtul <mpiasecki720@protonmail.com> Co-authored-by: vickydotbat <vickydotbat@tutamail.com>
This commit was merged in pull request #78.
This commit is contained in:
@@ -6,6 +6,7 @@ import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
@@ -193,14 +194,18 @@ func resolveIndexKey(artifactKey, outDir string) (string, error) {
|
||||
// checkArtifactKey fails closed when the key's embedded digest is not the
|
||||
// digest of the bytes being emitted. Publishing an index under the wrong key
|
||||
// silently pairs a manifest with the wrong artifact.
|
||||
func checkArtifactKey(artifactKey string, artifact []byte) error {
|
||||
// artifact is hashed by streaming, so a multi-gigabyte hak is never resident.
|
||||
func checkArtifactKey(artifactKey string, artifact io.Reader) error {
|
||||
base := path.Base(artifactKey)
|
||||
digest := strings.TrimSuffix(base, path.Ext(base))
|
||||
if len(digest) != 64 {
|
||||
return fmt.Errorf("artifact key %q does not name a sha256", artifactKey)
|
||||
}
|
||||
sum := sha256.Sum256(artifact)
|
||||
if got := hex.EncodeToString(sum[:]); got != digest {
|
||||
hash := sha256.New()
|
||||
if _, err := io.Copy(hash, artifact); err != nil {
|
||||
return fmt.Errorf("hash artifact: %w", err)
|
||||
}
|
||||
if got := hex.EncodeToString(hash.Sum(nil)); got != digest {
|
||||
return fmt.Errorf("artifact key %q names digest %s but the file hashes to %s", artifactKey, digest, got)
|
||||
}
|
||||
return nil
|
||||
|
||||
Reference in New Issue
Block a user