fix: invalid minimum benchmark time with b.loop

.github/workflows/ci.yml

@@ -56,7 +56,7 @@ jobs:
       - name: Run fork.sh scripts and verify no diffs
         run: |
           cd testing
-          ./fork.sh 1.25
+          ./fork.sh 1.25.4
           cd ..
 
           if ! git diff --exit-code testing/ > /dev/null 2>&1; then

example/setup_test.go

@@ -3,6 +3,7 @@ package example
 import (
 	"runtime"
 	"testing"
+	"time"
 )
 
 func BenchmarkLargeSetup(b *testing.B) {
@@ -50,3 +51,63 @@ func BenchmarkLargeSetupInLoop(b *testing.B) {
 	}
 	runtime.KeepAlive(result)
 }
+
+func BenchmarkWithOutlierMeasurementTraditional(b *testing.B) {
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		b.StopTimer()
+		time.Sleep(10 * time.Millisecond)
+		b.StartTimer()
+
+		time.Sleep(1 * time.Millisecond)
+	}
+}
+
+func BenchmarkWithOutlierMeasurementModern(b *testing.B) {
+	b.ResetTimer()
+	for b.Loop() {
+		b.StopTimer()
+		time.Sleep(10 * time.Millisecond)
+		b.StartTimer()
+
+		time.Sleep(1 * time.Millisecond)
+	}
+}
+
+func BenchmarkWithoutStartupModern(b *testing.B) {
+	time.Sleep(10 * time.Millisecond)
+	b.ResetTimer()
+	for b.Loop() {
+		time.Sleep(1 * time.Millisecond)
+	}
+}
+
+func BenchmarkWithoutStartupTraditional(b *testing.B) {
+	time.Sleep(10 * time.Millisecond)
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		time.Sleep(1 * time.Millisecond)
+	}
+}
+
+func BenchmarkWithStopTraditional(b *testing.B) {
+	time.Sleep(2 * time.Millisecond)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		time.Sleep(1 * time.Millisecond)
+	}
+
+	b.StopTimer()
+}
+
+func BenchmarkWithStopModern(b *testing.B) {
+	time.Sleep(2 * time.Millisecond)
+
+	b.ResetTimer()
+	for b.Loop() {
+		time.Sleep(1 * time.Millisecond)
+	}
+
+	b.StopTimer()
+}

go-runner/src/builder/snapshots/codspeed_go_runner__builder__discovery__tests__discover_benchmarks@example.snap

@@ -61,6 +61,54 @@ expression: packages
         "qualified_name": "benchmarklargesetupinloop_9576953055782987076.BenchmarkLargeSetupInLoop",
         "file_path": "setup_test.go",
         "is_external": false
+      },
+      {
+        "name": "BenchmarkWithOutlierMeasurementModern",
+        "module_path": "example",
+        "import_alias": "benchmarkwithoutliermeasurementmodern_9576953055782987076",
+        "qualified_name": "benchmarkwithoutliermeasurementmodern_9576953055782987076.BenchmarkWithOutlierMeasurementModern",
+        "file_path": "setup_test.go",
+        "is_external": false
+      },
+      {
+        "name": "BenchmarkWithOutlierMeasurementTraditional",
+        "module_path": "example",
+        "import_alias": "benchmarkwithoutliermeasurementtraditional_9576953055782987076",
+        "qualified_name": "benchmarkwithoutliermeasurementtraditional_9576953055782987076.BenchmarkWithOutlierMeasurementTraditional",
+        "file_path": "setup_test.go",
+        "is_external": false
+      },
+      {
+        "name": "BenchmarkWithStopModern",
+        "module_path": "example",
+        "import_alias": "benchmarkwithstopmodern_9576953055782987076",
+        "qualified_name": "benchmarkwithstopmodern_9576953055782987076.BenchmarkWithStopModern",
+        "file_path": "setup_test.go",
+        "is_external": false
+      },
+      {
+        "name": "BenchmarkWithStopTraditional",
+        "module_path": "example",
+        "import_alias": "benchmarkwithstoptraditional_9576953055782987076",
+        "qualified_name": "benchmarkwithstoptraditional_9576953055782987076.BenchmarkWithStopTraditional",
+        "file_path": "setup_test.go",
+        "is_external": false
+      },
+      {
+        "name": "BenchmarkWithoutStartupModern",
+        "module_path": "example",
+        "import_alias": "benchmarkwithoutstartupmodern_9576953055782987076",
+        "qualified_name": "benchmarkwithoutstartupmodern_9576953055782987076.BenchmarkWithoutStartupModern",
+        "file_path": "setup_test.go",
+        "is_external": false
+      },
+      {
+        "name": "BenchmarkWithoutStartupTraditional",
+        "module_path": "example",
+        "import_alias": "benchmarkwithoutstartuptraditional_9576953055782987076",
+        "qualified_name": "benchmarkwithoutstartuptraditional_9576953055782987076.BenchmarkWithoutStartupTraditional",
+        "file_path": "setup_test.go",
+        "is_external": false
       }
     ]
   },

go-runner/src/snapshots/codspeed_go_runner__integration_tests__assert_results_snapshots@example.snap

@@ -190,6 +190,72 @@ expression: results
           "max_rounds": null
         },
         "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithOutlierMeasurementModern",
+        "uri": "example/setup_test.go::BenchmarkWithOutlierMeasurementModern",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithOutlierMeasurementTraditional",
+        "uri": "example/setup_test.go::BenchmarkWithOutlierMeasurementTraditional",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithStopModern",
+        "uri": "example/setup_test.go::BenchmarkWithStopModern",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithStopTraditional",
+        "uri": "example/setup_test.go::BenchmarkWithStopTraditional",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithoutStartupModern",
+        "uri": "example/setup_test.go::BenchmarkWithoutStartupModern",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
+      },
+      {
+        "name": "BenchmarkWithoutStartupTraditional",
+        "uri": "example/setup_test.go::BenchmarkWithoutStartupTraditional",
+        "config": {
+          "warmup_time_ns": null,
+          "min_round_time_ns": null,
+          "max_time_ns": null,
+          "max_rounds": null
+        },
+        "stats": "[stats]"
       }
     ]
   },

testing/fork.sh

@@ -94,11 +94,12 @@ backup_files "${CODSPEED_FILES[@]}"
 
 # We need to copy the testing/ package:
 rm -rf go testing internal
-git clone -b "release-branch.go${GO_VERSION}" --depth 1 https://github.com/golang/go/
+git clone -b "go${GO_VERSION}" --depth 1 https://github.com/golang/go/
 cp -r go/src/testing testing/
 
 # Copy all required internal packages. We need them to have a clean `go mod tidy` output.
 copy_internal_packages "cpu" "fuzz" "goarch" "race" "sysinfo" "testlog" "testenv" "syscall/windows" "godebug" "synctest" "bisect" "godebugs" "cfg" "platform" "diff" "txtar"
+rm -rf go
 
 # Replace all `"internal/*"` imports with 'github.com/CodSpeedHQ/codspeed-go/testing/internal/'
 find . -type f -name "*.go" -exec sed -i 's|"internal/|"github.com/CodSpeedHQ/codspeed-go/testing/internal/|g' {} +
@@ -121,6 +122,8 @@ find . -type f -name "*.go" -not -name "*_test.go" -exec sed -i 's|"testing"|tes
 restore_files "${CODSPEED_FILES[@]}"
 
 apply_patch "patches/benchmark_stopbenchmark_fail.patch" 10 ".."
+apply_patch "patches/benchmark_stoptimer_mitigation.patch" 10 ".."
+
 
 # Run pre-commit and format the code
 go fmt ./... > /dev/null 2>&1 || true

testing/patches/benchmark_stoptimer_mitigation.patch

@@ -0,0 +1,78 @@
+diff --git a/testing/testing/benchmark.go b/testing/testing/benchmark.go
+index 8fdea90..4947e11 100644
+--- a/testing/testing/benchmark.go
++++ b/testing/testing/benchmark.go
+@@ -93,6 +93,10 @@ type codspeed struct {
+ 	startTimestamp  uint64
+ 	startTimestamps []uint64
+ 	stopTimestamps  []uint64
++
++	// Indicates whether a measurement has been saved already. This aims to prevent saving measurements
++	// twice, because `b.Loop()` saves them internally as well but is also called from runN
++	savedMeasurement bool
+ }
+
+ // B is a type passed to [Benchmark] functions to manage benchmark
+@@ -160,6 +164,7 @@ func (b *B) StartTimerWithoutMarker() {
+ 		// b.startBytes = memStats.TotalAlloc
+ 		b.start = highPrecisionTimeNow()
+ 		b.timerOn = true
++		b.savedMeasurement = false
+ 		// b.loop.i &^= loopPoisonTimer
+ 	}
+ }
+@@ -186,14 +191,32 @@ func (b *B) StopTimerWithoutMarker() {
+ 		b.timerOn = false
+ 		// If we hit B.Loop with the timer stopped, fail.
+ 		// b.loop.i |= loopPoisonTimer
++	}
++}
+
+-		// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
+-		// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
+-		// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
+-		// the number of iterations for this round.
+-		b.codspeedItersPerRound = append(b.codspeedItersPerRound, max(int64(b.N), 1))
+-		b.codspeedTimePerRoundNs = append(b.codspeedTimePerRoundNs, timeSinceStart)
++func (b *B) SaveMeasurement() {
++	if b.savedMeasurement {
++		return
+ 	}
++	b.savedMeasurement = true
++
++	// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
++	// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
++	// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
++	// the number of iterations for this round.
++	timeSinceStart := highPrecisionTimeSince(b.start)
++
++	// If this gets called from b.Loop(), we have to take the duration compared to the previous StartTimer,
++	// if it's called from runN, we can use b.duration
++	duration := time.Duration(0)
++	if b.N == 0 {
++		duration = timeSinceStart
++	} else {
++		duration = b.duration
++	}
++
++	b.codspeedItersPerRound = append(b.codspeedItersPerRound, max(int64(b.N), 1))
++	b.codspeedTimePerRoundNs = append(b.codspeedTimePerRoundNs, duration)
+ }
+
+ func (b *B) StopTimer() {
+@@ -296,6 +319,7 @@ func (b *B) __codspeed_root_frame__runN(n int) {
+ 	b.ResetTimer()
+ 	b.StartTimer()
+ 	b.benchFunc(b)
++	b.SaveMeasurement()
+ 	b.StopTimer()
+ 	b.previousN = n
+ 	b.previousDuration = b.duration
+@@ -614,6 +638,7 @@ func (b *B) loopSlowPath() bool {
+ // whereas b.N-based benchmarks must run the benchmark function (and any
+ // associated setup and cleanup) several times.
+ func (b *B) Loop() bool {
++	b.SaveMeasurement()
+ 	b.StopTimerWithoutMarker()
+ 	// This is written such that the fast path is as fast as possible and can be
+ 	// inlined.

testing/testing/benchmark.go

@@ -93,6 +93,10 @@ type codspeed struct {
 	startTimestamp  uint64
 	startTimestamps []uint64
 	stopTimestamps  []uint64
+
+	// Indicates whether a measurement has been saved already. This aims to prevent saving measurements
+	// twice, because `b.Loop()` saves them internally as well but is also called from runN
+	savedMeasurement bool
 }
 
 // B is a type passed to [Benchmark] functions to manage benchmark
@@ -160,6 +164,7 @@ func (b *B) StartTimerWithoutMarker() {
 		// b.startBytes = memStats.TotalAlloc
 		b.start = highPrecisionTimeNow()
 		b.timerOn = true
+		b.savedMeasurement = false
 		// b.loop.i &^= loopPoisonTimer
 	}
 }
@@ -186,14 +191,32 @@ func (b *B) StopTimerWithoutMarker() {
 		b.timerOn = false
 		// If we hit B.Loop with the timer stopped, fail.
 		// b.loop.i |= loopPoisonTimer
+	}
+}
 
-		// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
-		// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
-		// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
-		// the number of iterations for this round.
-		b.codspeedItersPerRound = append(b.codspeedItersPerRound, max(int64(b.N), 1))
-		b.codspeedTimePerRoundNs = append(b.codspeedTimePerRoundNs, timeSinceStart)
+func (b *B) SaveMeasurement() {
+	if b.savedMeasurement {
+		return
 	}
+	b.savedMeasurement = true
+
+	// For b.N loops: This will be called in runN which sets b.N to the number of iterations.
+	// For b.Loop() loops: loopSlowPath sets b.N to 0 to prevent b.N loops within b.Loop. However, since
+	// we're starting/stopping the timer for each iteration in the b.Loop() loop, we can use 1 as
+	// the number of iterations for this round.
+	timeSinceStart := highPrecisionTimeSince(b.start)
+
+	// If this gets called from b.Loop(), we have to take the duration compared to the previous StartTimer,
+	// if it's called from runN, we can use b.duration
+	duration := time.Duration(0)
+	if b.N == 0 {
+		duration = timeSinceStart
+	} else {
+		duration = b.duration
+	}
+
+	b.codspeedItersPerRound = append(b.codspeedItersPerRound, max(int64(b.N), 1))
+	b.codspeedTimePerRoundNs = append(b.codspeedTimePerRoundNs, duration)
 }
 
 func (b *B) StopTimer() {
@@ -296,6 +319,7 @@ func (b *B) __codspeed_root_frame__runN(n int) {
 	b.ResetTimer()
 	b.StartTimer()
 	b.benchFunc(b)
+	b.SaveMeasurement()
 	b.StopTimer()
 	b.previousN = n
 	b.previousDuration = b.duration
@@ -614,6 +638,7 @@ func (b *B) loopSlowPath() bool {
 // whereas b.N-based benchmarks must run the benchmark function (and any
 // associated setup and cleanup) several times.
 func (b *B) Loop() bool {
+	b.SaveMeasurement()
 	b.StopTimerWithoutMarker()
 	// This is written such that the fast path is as fast as possible and can be
 	// inlined.