Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: clarify that b.N does not change during execution of the benchmark function #67137

Closed
dolmen opened this issue May 2, 2024 · 4 comments
Labels
Documentation Issues describing a change to documentation. help wanted
Milestone

Comments

@dolmen
Copy link
Contributor

dolmen commented May 2, 2024

The testing documentation clearly mentions that the loop boundary variable might change during execution:

During benchmark execution, b.N is adjusted until the benchmark function lasts long enough to be timed reliably.

I noticed that range over integer loop have different behavior than for with clause loops when the loop boundary is changed by the loop body:
https://go.dev/play/p/cObUT32FcXc

	n := 4
	for i := 0; i < n; i++ {
		fmt.Println(n)
		n--
	}

	fmt.Println("----")

	n = 4
	for range n {
		fmt.Println(n)
		n--
	}
4
3
----
4
3
2
1

I have not found this difference in behavior explicitely mentioned in the Go specification. The following sentence speaks about the first evaluation of the range expression, but doesn't mention later evaluations.

The range expression x is evaluated once before beginning the loop, with one exception: if at most one iteration variable is present and len(x) is constant, the range expression is not evaluated.

I wonder how a benchmark would be affected by the use of range-over-integer instead of the documented style.

I have seen no clear mention of forbidding the use of range-over-integer with b.N for benchmarks. I have not seen either an explicit mention that range-over-int is compatible with testing.B benchmarks which makes me wonder if this has been considered. I also did a code search on range b.N in the Go repository and the only result is a test in the database/sql package just added less that 2 months ago by @bradfitz in CL 572119. No tests in testing.

So it would be helpful to have explicit documentation about the use of b.N with range-over-integer.

If this is in fact an incompatibility, it would be helpful to have a go vet alert about use of range-over-int on testing.B.N.

Go version

go version go1.22.2 darwin/arm64

Output of go env in your module/workspace:

N/A (reproducible on play.golang.org)

What did you see happen?

$ go doc -all testing | grep -c 'range b.N'
0

What did you expect to see?

$ go doc -all testing | grep -c 'range b.N'
1
@jub0bs
Copy link

jub0bs commented May 2, 2024

Following a discussion on Gophers Slack and a casual inspection of testing's source code, I don't think b.N changes during the execution of a benchmark subject. However, I agree with @dolmen that the documentation is ambiguous/misleading in that respect.

@ianlancetaylor ianlancetaylor added the Documentation Issues describing a change to documentation. label May 2, 2024
@ianlancetaylor ianlancetaylor changed the title testing: mention explicitely (in-)compatibility of writing benchmarks using range-over-integer loops testing: clarify that b.N does not change during benchmark execution May 2, 2024
@ianlancetaylor ianlancetaylor added this to the Backlog milestone May 2, 2024
@ianlancetaylor ianlancetaylor changed the title testing: clarify that b.N does not change during benchmark execution testing: clarify that b.N does not change during execution of the benchmark function May 2, 2024
@dolmen
Copy link
Contributor Author

dolmen commented May 2, 2024

I've been a fulltime Go developer for 9 years. I've read the documentation about benchmarks multiple times. I never thought that the whole benchmark function was called multiple times because this is different that Test functions which are called just once. Instead, because of that sentence in the documentation, I thought that b.N was changed dynamically (with some supposed magic locking that I didn't investigate: after all code coverage via source instrumentation is already magic of go test).

@Jorropo
Copy link
Member

Jorropo commented May 2, 2024

I have not found this difference in behavior explicitely mentioned in the Go specification. The following sentence speaks about the first evaluation of the range expression, but doesn't mention later evaluations.

The range expression x is evaluated once before beginning the loop, with one exception: if at most one iteration variable is present and len(x) is constant, the range expression is not evaluated.

This is explicitly mentioned: « once before beginning the loop » describe precisely that behavior.
I'm not saying this shouldn't be made clearer or something, just it's working as expected and has been for years and changing this would be a language breaking change.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/582835 mentions this issue: testing: improve the documentation around b.N

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation. help wanted
Projects
None yet
Development

No branches or pull requests

5 participants