Uh oh!
There was an error while loading. Please reload this page.
Migrate b.N benchmark loops to b.Loop() - #5305
Merged
Merged
Conversation
Bump Go to `go1.26.0` / `toolchain go1.26.3` across all four modules. This cleans up an inconsistency: `go.mod`, `tools/task/go.mod`, and `bundle/internal/tf/codegen/go.mod` were sitting at `go 1.25.8` while `tools/go.mod` had drifted back to `go 1.25.0`. All four are now in sync. Release notes: https://go.dev/doc/go1.26 Co-authored-by: Isaac
Bumping the `go` directive to 1.26.0 unlocks two modernize analyzers that
were silent on 1.25:
* stditerators - prefer reflect.Type.Fields()/Methods() and
reflect.Value.Fields()/Methods() over the classic
NumField()/Field(i) loop pattern.
* newexpr - replace local `*T` helpers like `func intPtr(v int) *int
{ return &v }` (and their callers) with Go 1.26's `new(expr)`.
This commit is the verbatim output of `golangci-lint run --fix ./...`,
applied at the same time as the toolchain bump so CI doesn't fail the
moment the bump lands.
A few `field := field` style redeclarations the fixer leaves behind are
harmless (the loop variable is fresh per-iteration since Go 1.22). Leaving
those for a follow-up cleanup rather than expanding the diff here.
Co-authored-by: IsaacFollow-up to the previous commit, which the previous commit message deferred. Two cosmetic clean-ups: 1. Remove 14 redundant `x := x` redeclarations the modernize fixer left inside `for x := range t.Fields()` (or `.Methods()`) loops. The loop variable is already fresh per-iteration since Go 1.22, so the shadow was a no-op kept only to preserve the variable name from the old `field := t.Field(i)` pattern. 2. Remove dead `*Ptr` helper functions whose call sites were inlined to `new(expr)`. The fixer added `//go:fix inline` directives and rewrote the bodies to `return new(v)`, leaving the functions themselves with zero callers (except `int64Ptr`, where `new(700)` would yield `*int` instead of `*int64`; here we rewrite the 20 callers to `new(int64(N))` and drop the helper too). * bundle/docsgen/nodes_test.go: drop strPtr * libs/apps/runlocal/spec_test.go: drop stringPtr * libs/structs/structaccess/convert_test.go: drop stringPtr, intPtr, float64Ptr, boolPtr * cmd/pipelines/history_test.go: rewrite int64Ptr callers, drop helper Co-authored-by: Isaac
Co-authored-by: Isaac
testing.B.Loop() was added in Go 1.24 with a performance regression that made it prevent inlining; that was fixed in Go 1.26. There's no longer any reason to use 'for i := 0; i < b.N; i++' — b.Loop() handles the loop counter, correct keep-alive semantics, and timer setup automatically. This commit adds the rule only; lint will fail on existing benchmarks. The follow-up commit migrates them. Co-authored-by: Isaac
b.Loop() was added in Go 1.24 but had an inlining regression that's fixed in Go 1.26. The new form is strictly better: automatic timer setup, correct keep-alive of the loop body, and inlineable. b.ResetTimer() calls immediately before the loop are no longer needed. Co-authored-by: Isaac
Collaborator
Commit: ff9c1ad |
denik
approved these changes
May 21, 2026
denik
left a comment
Contributor
There was a problem hiding this comment.
nit: would be nice to include as a comment output of before and after, since these are manually run.
pietern
commented
May 26, 2026
ContributorAuthor
I ran a before/after manually and there was no meaningful difference. |
pietern
enabled auto-merge
May 26, 2026 13:24
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
eng-dev-ecosystem-bot
commented
May 26, 2026
Collaborator
Commit: 9486bdf |
eng-dev-ecosystem-bot
commented
May 26, 2026
Collaborator
Commit: 9486bdf |
denik pushed a commit
that referenced
this pull request
May 28, 2026
Stacked on top of #5302. Go 1.26 fixed the inlining regression that made `b.Loop()` worse than `b.N` on 1.24/1.25. Migrates the 4 remaining loops and adds a ruleguard rule. Redundant `b.ResetTimer()`/`b.StopTimer()` around each loop are removed too. This pull request and its description were written by Isaac.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of #5302.
Go 1.26 fixed the inlining regression that made
b.Loop()worse thanb.Non 1.24/1.25. Migrates the 4 remaining loops and adds a ruleguard rule. Redundantb.ResetTimer()/b.StopTimer()around each loop are removed too.This pull request and its description were written by Isaac.