Skip to content

test(format): measure realistic allocations in Format/Append benchmarks#224

Merged
ReneWerner87 merged 1 commit into
masterfrom
test/format-bench-bloop
Jun 23, 2026
Merged

test(format): measure realistic allocations in Format/Append benchmarks#224
ReneWerner87 merged 1 commit into
masterfrom
test/format-bench-bloop

Conversation

@ReneWerner87

Copy link
Copy Markdown
Member

Summary

The Format* / Append* benchmarks discarded their result with _ =. For
inlinable functions such as FormatUint, escape analysis then kept the returned
string on the stack, reporting a misleading 0 allocs/op that real callers,
which retain the returned string, never observe.

This switches the benchmark loops to b.Loop() (Go 1.24+). b.Loop() keeps call
results alive, so the value escapes and the benchmark measures the true allocation
cost - giving a fair fiber vs strconv comparison. As a bonus it resolves the
modernize linter's bloop suggestion for these loops.

Measured impact (methodology fix, not a perf change)

FormatUint/medium:

B/op allocs/op
before (_ = discard) 0 0 (misleading)
after (b.Loop()) 16 1 (the real cost)

Genuine fiber wins stay visible and honest: the small-value cache (e.g.
FormatInt/small_neg: 0 allocs vs strconv's 1) and FormatUint32 (CPU). The
apparent advantage for small positive values was itself a measurement artifact
and correctly disappears under b.Loop().

Notes

  • No production code changed - only format_test.go.
  • gofumpt, go vet and golangci-lint v2.6.1 are clean for the changed file.

🤖 Generated with Claude Code

@ReneWerner87 ReneWerner87 requested a review from a team as a code owner June 23, 2026 20:50
@ReneWerner87 ReneWerner87 requested review from efectn, gaby and sixcolors and removed request for a team June 23, 2026 20:50
The Format*/Append* benchmarks discarded their result with "_ =". For
inlinable functions such as FormatUint, escape analysis then kept the
returned string on the stack, reporting a misleading 0 allocs/op that
real callers, which retain the string, never observe.

Switch the loops to b.Loop() (Go 1.24+). b.Loop keeps the call results
alive so the value escapes, and the benchmark measures the true
allocation cost, giving a fair fiber-vs-strconv comparison. It also
resolves the modernize linter's "bloop" suggestion for these loops.

Measured (FormatUint/medium):
  before, "_ =" discard: 0 B/op, 0 allocs/op (misleading)
  after,  b.Loop():      16 B/op, 1 allocs/op (the real cost)

No production code changed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.02%. Comparing base (d85880d) to head (2f1a488).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #224   +/-   ##
=======================================
  Coverage   84.02%   84.02%           
=======================================
  Files          14       14           
  Lines        1158     1158           
=======================================
  Hits          973      973           
  Misses        154      154           
  Partials       31       31           
Flag Coverage Δ
unittests 84.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ReneWerner87 ReneWerner87 force-pushed the test/format-bench-bloop branch from 21fb997 to 2f1a488 Compare June 23, 2026 20:52
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ReneWerner87, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 56 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 465c2cc8-177b-4d76-94b7-a9848b2a0f32

📥 Commits

Reviewing files that changed from the base of the PR and between 2e58bec and 2f1a488.

📒 Files selected for processing (1)
  • format_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/format-bench-bloop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ReneWerner87 ReneWerner87 merged commit cf60a2e into master Jun 23, 2026
16 of 18 checks passed
@ReneWerner87 ReneWerner87 deleted the test/format-bench-bloop branch June 23, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant