Skip to content

Commit 5331397

Browse files
committed
fix(testing): incomplete reading of benchmark time in b.N loops
1 parent 0ee1cab commit 5331397

3 files changed

Lines changed: 46 additions & 2 deletions

File tree

testing/fork.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ restore_files "${CODSPEED_FILES[@]}"
124124
apply_patch "patches/benchmark_stopbenchmark_fail.patch" 10 ".."
125125
apply_patch "patches/benchmark_stoptimer_mitigation.patch" 10 ".."
126126
apply_patch "patches/benchmark_benchmarkers_bloop.patch" 10 ".."
127+
apply_patch "patches/benchmark_savemeasurement_bug.patch" 10 ".."
127128

128129

129130
# Run pre-commit and format the code
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
diff --git a/testing/testing/benchmark.go b/testing/testing/benchmark.go
2+
index 0040973..883976e 100644
3+
--- a/testing/testing/benchmark.go
4+
+++ b/testing/testing/benchmark.go
5+
@@ -200,6 +200,12 @@ func (b *B) SaveMeasurement() {
6+
}
7+
b.savedMeasurement = true
8+
9+
+ // WARN: This function must not be called if the timer is on, because we
10+
+ // would read an incomplete b.duration value.
11+
+ if b.timerOn {
12+
+ panic("SaveMeasurement called with timer on")
13+
+ }
14+
+
15+
// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
16+
// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
17+
// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
18+
@@ -311,8 +317,8 @@ func (b *B) __codspeed_root_frame__runN(n int) {
19+
b.ResetTimer()
20+
b.StartTimer()
21+
b.benchFunc(b)
22+
- b.SaveMeasurement()
23+
b.StopTimer()
24+
+ b.SaveMeasurement()
25+
b.previousN = n
26+
b.previousDuration = b.duration
27+
28+
@@ -641,8 +647,8 @@ func (b *B) loopSlowPath() bool {
29+
// whereas b.N-based benchmarks must run the benchmark function (and any
30+
// associated setup and cleanup) several times.
31+
func (b *B) Loop() bool {
32+
- b.SaveMeasurement()
33+
b.StopTimerWithoutMarker()
34+
+ b.SaveMeasurement()
35+
// This is written such that the fast path is as fast as possible and can be
36+
// inlined.
37+
//

testing/testing/benchmark.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ func (b *B) SaveMeasurement() {
200200
}
201201
b.savedMeasurement = true
202202

203+
// WARN: This function must not be called if the timer is on, because we
204+
// would read an incomplete b.duration value.
205+
if b.timerOn {
206+
panic("SaveMeasurement called with timer on")
207+
}
208+
203209
// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
204210
// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
205211
// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
@@ -311,8 +317,8 @@ func (b *B) __codspeed_root_frame__runN(n int) {
311317
b.ResetTimer()
312318
b.StartTimer()
313319
b.benchFunc(b)
314-
b.SaveMeasurement()
315320
b.StopTimer()
321+
b.SaveMeasurement()
316322
b.previousN = n
317323
b.previousDuration = b.duration
318324

@@ -641,8 +647,8 @@ func (b *B) loopSlowPath() bool {
641647
// whereas b.N-based benchmarks must run the benchmark function (and any
642648
// associated setup and cleanup) several times.
643649
func (b *B) Loop() bool {
644-
b.SaveMeasurement()
645650
b.StopTimerWithoutMarker()
651+
b.SaveMeasurement()
646652
// This is written such that the fast path is as fast as possible and can be
647653
// inlined.
648654
//

0 commit comments

Comments
 (0)