Skip to content

Commit 9486bdf

Browse files
authored
Migrate b.N benchmark loops to b.Loop() (#5305)
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.
1 parent 4ac8f01 commit 9486bdf

4 files changed

Lines changed: 19 additions & 8 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package gorules
2+
3+
import "github.com/quasilyte/go-ruleguard/dsl"
4+
5+
// UseBenchmarkLoop detects classic benchmark loops over b.N and suggests
6+
// b.Loop() (Go 1.24+; the inlining regression was fixed in 1.26 so there's
7+
// no longer a reason to keep b.N-based loops).
8+
func UseBenchmarkLoop(m dsl.Matcher) {
9+
m.Match(
10+
`for $_ := range $b.N`,
11+
`for range $b.N`,
12+
).
13+
Where(m["b"].Type.Is("*testing.B")).
14+
Report(`Use 'for $b.Loop()' instead of looping over $b.N (Go 1.24+, performance-correct in 1.26+)`)
15+
}

libs/structs/structdiff/bench_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ func bench(b *testing.B, job1, job2 string) {
1717

1818
total := 0
1919

20-
b.ResetTimer()
21-
for range b.N {
20+
for b.Loop() {
2221
changes, err := GetStructDiff(&x, &y, nil)
2322
if err != nil {
2423
b.Fatalf("error: %s", err)
2524
}
2625
total += len(changes)
2726
}
28-
b.StopTimer()
2927

3028
b.Logf("Total: %d / %d", total, b.N)
3129
}

libs/structs/structtag/jsontag_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,15 @@ var benchTags = []JSONTag{
5959
}
6060

6161
func BenchmarkJSONTagName(b *testing.B) {
62-
for range b.N {
62+
for b.Loop() {
6363
for _, tag := range benchTags {
6464
tag.Name()
6565
}
6666
}
6767
}
6868

6969
func BenchmarkJSONTagOmitEmpty(b *testing.B) {
70-
for range b.N {
70+
for b.Loop() {
7171
for _, tag := range benchTags {
7272
tag.OmitEmpty()
7373
}

libs/structs/structwalk/walktype_bench_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,14 @@ func countFields(typ reflect.Type) (int, error) {
2121
func benchmarkWalkType(b *testing.B, tt reflect.Type) {
2222
total := 0
2323

24-
b.ResetTimer()
25-
for range b.N {
24+
for b.Loop() {
2625
count, err := countFields(tt)
2726
if err != nil {
2827
b.Fatalf("WalkType failed: %v", err)
2928
}
3029
total += count
3130
}
3231

33-
b.StopTimer()
3432
// Root now correctly includes embedded struct fields, so it has many more fields than JobSettings
3533
// (3,487 vs 533) because it contains JobSettings plus other resource types and config fields
3634
b.Logf("Counted fields in %s: %d (%d/%d)", tt, total/b.N, total, b.N)

0 commit comments

Comments
 (0)