Skip to content

Commit aaf9b46

Browse files
aclementsgopherbot
authored andcommitted
testing: streamline logic in loopSlowPath
There's a fair amount of duplication of logic between various return branches of loopSlowPath and stopOrScaleBLoop. Restructure these so there's a single "keep going" path and a single "we're done" path. Change-Id: I38e4c7a616f8bd7707f3ca886f38ff21dbd78b6b Reviewed-on: https://go-review.googlesource.com/c/go/+/659658 Auto-Submit: Austin Clements <[email protected]> Reviewed-by: Junyang Shao <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
1 parent 5918101 commit aaf9b46

File tree

1 file changed

+30
-18
lines changed

1 file changed

+30
-18
lines changed

src/testing/benchmark.go

+30-18
Original file line numberDiff line numberDiff line change
@@ -391,11 +391,7 @@ func (b *B) ReportMetric(n float64, unit string) {
391391
func (b *B) stopOrScaleBLoop() bool {
392392
t := b.Elapsed()
393393
if t >= b.benchTime.d {
394-
// Stop the timer so we don't count cleanup time
395-
b.StopTimer()
396-
// Commit iteration count
397-
b.N = int(b.loop.n)
398-
b.loop.done = true
394+
// We've reached the target
399395
return false
400396
}
401397
// Loop scaling
@@ -407,7 +403,6 @@ func (b *B) stopOrScaleBLoop() bool {
407403
// in big trouble.
408404
panic("loop iteration target overflow")
409405
}
410-
b.loop.i++
411406
return true
412407
}
413408

@@ -421,31 +416,48 @@ func (b *B) loopSlowPath() bool {
421416
}
422417

423418
if b.loop.n == 0 {
424-
// If it's the first call to b.Loop() in the benchmark function.
425-
// Allows more precise measurement of benchmark loop cost counts.
426-
// Also initialize target to 1 to kick start loop scaling.
427-
b.loop.n = 1
419+
// It's the first call to b.Loop() in the benchmark function.
420+
if b.benchTime.n > 0 {
421+
// Fixed iteration count.
422+
b.loop.n = uint64(b.benchTime.n)
423+
} else {
424+
// Initialize target to 1 to kick start loop scaling.
425+
b.loop.n = 1
426+
}
428427
// Within a b.Loop loop, we don't use b.N (to avoid confusion).
429428
b.N = 0
430-
b.loop.i++
431429
b.ResetTimer()
430+
431+
// Start the next iteration.
432+
b.loop.i++
432433
return true
433434
}
434-
// Handles fixed iterations case
435+
436+
// Should we keep iterating?
437+
var more bool
435438
if b.benchTime.n > 0 {
436-
if b.loop.n < uint64(b.benchTime.n) {
437-
b.loop.n = uint64(b.benchTime.n)
438-
b.loop.i++
439-
return true
439+
// The iteration count is fixed, so we should have run this many and now
440+
// be done.
441+
if b.loop.i != uint64(b.benchTime.n) {
442+
// We shouldn't be able to reach the slow path in this case.
443+
panic(fmt.Sprintf("iteration count %d < fixed target %d", b.loop.i, b.benchTime.n))
440444
}
445+
more = false
446+
} else {
447+
// Handle fixed time case
448+
more = b.stopOrScaleBLoop()
449+
}
450+
if !more {
441451
b.StopTimer()
442452
// Commit iteration count
443453
b.N = int(b.loop.n)
444454
b.loop.done = true
445455
return false
446456
}
447-
// Handles fixed time case
448-
return b.stopOrScaleBLoop()
457+
458+
// Start the next iteration.
459+
b.loop.i++
460+
return true
449461
}
450462

451463
// Loop returns true as long as the benchmark should continue running.

0 commit comments

Comments
 (0)