Skip to content

Reduce allocations in shake256.NewDigestForContent#4505

Merged
emcfarlane merged 3 commits intomainfrom
ed/shake256-pool
Apr 28, 2026
Merged

Reduce allocations in shake256.NewDigestForContent#4505
emcfarlane merged 3 commits intomainfrom
ed/shake256-pool

Conversation

@emcfarlane
Copy link
Copy Markdown
Contributor

@emcfarlane emcfarlane commented Apr 24, 2026

Both shake256.NewDigestForContent and bufimage.readObjectCloserToSourceFile use io.Copy into an in-memory writer. With no WriterTo/ReaderFrom fast path available, io.Copy allocates its 32 KiB scratch buffer per call, much larger then many protobuf files.

Updated to pass an explicit 1 KiB buffer to io.CopyBuffer. This reduces bytes allocated.

Two new benchmarks (BenchmarkBucketToDigest, BenchmarkBuildImage) cover the digest and image-build paths over googleapis. Per pass:

  • BucketToDigest: 78.9 MiB -> 15.4 MiB allocated (-80%)
  • BuildImage: 578.6 MiB -> 544.4 MiB allocated (-6%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 27, 2026, 4:15 PM

@emcfarlane emcfarlane requested review from doriable and pkwarren April 24, 2026 21:54
Copy link
Copy Markdown
Member

@pkwarren pkwarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a benchmark for this, or is it covered by other benchmarks? I'd be interested to see the effect of say building a manifest/module for the buf CLI's own proto/ sources.

@emcfarlane
Copy link
Copy Markdown
Contributor Author

Updated to remove the pools and use a small buffer. This is very similar in perf but this change is maybe not worth it. Its a minimal improvement only improving the B/op but not a real impact in cpu or allocations.

$ benchstat new.txt old.txt pool.txt
goos: darwin
goarch: arm64
pkg: github.com/bufbuild/buf/private/pkg/cas
cpu: Apple M2
                 │    new.txt    │             old.txt             │            pool.txt             │
                 │    sec/op     │    sec/op     vs base           │    sec/op     vs base           │
BucketToDigest-8   105.08m ± ∞ ¹   97.74m ± ∞ ¹  ~ (p=0.100 n=3) ²   99.71m ± ∞ ¹  ~ (p=0.100 n=3) ²
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                 │    new.txt    │             old.txt              │             pool.txt             │
                 │     B/op      │     B/op       vs base           │     B/op       vs base           │
BucketToDigest-8   15.37Mi ± ∞ ¹   78.92Mi ± ∞ ¹  ~ (p=0.100 n=3) ²   12.91Mi ± ∞ ¹  ~ (p=0.100 n=3) ²
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                 │    new.txt    │             old.txt              │            pool.txt             │
                 │   allocs/op   │   allocs/op    vs base           │  allocs/op    vs base           │
BucketToDigest-8   100.15k ± ∞ ¹   100.15k ± ∞ ¹  ~ (p=1.000 n=3) ²   95.99k ± ∞ ¹  ~ (p=0.100 n=3) ²
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

@emcfarlane emcfarlane requested a review from pkwarren April 27, 2026 15:37
Comment thread private/bufpkg/bufimage/build_image_bench_test.go Outdated
Comment thread private/bufpkg/bufimage/module_file_resolver.go
@emcfarlane emcfarlane merged commit f781a0e into main Apr 28, 2026
10 checks passed
@emcfarlane emcfarlane deleted the ed/shake256-pool branch April 28, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants