testing: better documentation for RunParallel

[dvyukov]

From golang-dev:

Before the 1.3 release, could the documentation be improved on RunParallel (and probably
by extension SetParallel). I was looking at it today and I am confused on the intended
usage.

For my case, I believe I don't need to use them, but I don't think the documentation for
RunParallel and SetParallel gives a good enough sense for when they should be used.
RunParallel says "It creates multiple goroutines and distributes b.N iterations
among them". Is the purpose to make the benchmark go faster by executing it
concurrently? Is the point to measure how concurrently executing processes interact with
one another? In addition to an answer to that question, it would be great to add more
prose explaining the kinds of benchmarks where one would want to use SetParallelism and
RunParallel. All SetParallelism says is that you shouldn't do it if you are CPU-bound.
Adding text like "SetParallelism is useful, say, to measure the memory used by
spawning many worker goroutines" (assuming that's correct).

[dvyukov]

Comment 1:

> Is the point to measure how concurrently executing processes interact with one another?
Yes, this is the main use case.
E.g. if you are doing several concurrent http requests using a single http.Client, does
it scale linearly with GOMAXPROCS? or there is some bottleneck in the shared Client?
> it would be great to add more prose explaining the kinds of benchmarks where one would
want to use SetParallelism
If you would write the benchmark w/o using RunParallel, how many goroutines would you
create? Use SetParallelism to create the same number of goroutines.
For a CPU-bound benchmark higher parallelism will make little difference.
However, for if you are writing a benchmark that uses http.Client, (1) it can go faster
with higher parallelism, (2) set parallelism to what you use in real program, the value
that is interesting to you. E.g. if in real program you send 100*GOMAXPROCS concurrent
requests at once, do the same in the benchmark.
That was my ideas behind RunParallel/SetParallelism.

[josharian]

Comment 2:

I took a pass at this but struggled to find improvements, so I'd like a bit more input.
Here's are some slightly tweaked RunParallel docs:
// RunParallel runs a benchmark in parallel.
// Use RunParallel to measure how a benchmark scales with increased
// concurrency. It is usually used with the go test -cpu flag.
//
// RunParallel creates multiple goroutines and distributes b.N iterations among them.
// The number of goroutines defaults to GOMAXPROCS. To increase parallelism for
// non-CPU-bound benchmarks, call SetParallelism before RunParallel.
//
// The body function will be run in each goroutine. It should set up any
// goroutine-local state and then iterate until pb.Next returns false.
// It should not use the StartTimer, StopTimer, or ResetTimer functions,
// because they have global effect.
Brendan, would that clear things up sufficiently? What is still missing?
I'm really unsure what else to add to SetParallelism. It doesn't seem obviously valuable
to list the non-CPU ways a benchmark could be bound.
Dmitry, can you think of a short, simple benchmark that is obviously not CPU-bound that
I could add as example code for SetParallelism?

[dvyukov]

Comment 3:

> Dmitry, can you think of a short, simple benchmark that is obviously not CPU-bound
that I could add as example code for SetParallelism?
HTTP server is not CPU bound with 1 request per core. There is another reason to
benchmark it with higher parallelism -- it's more realistic.

[josharian]

Comment 4:

Brendan suggested these docs on golang-dev:
// RunParallel measures the effect of concurrent execution on the
// performance of a benchmark by spawning multiple goroutines
// and distributing the benchmark iterations among them. RunParallel
// will usually be used with the go test -cpu flag.
//
// The body function will be run in each goroutine. It should set up any 
// goroutine-local state and iterate until pb.Next returns false. It should 
// not use the StartTimer, StopTimer, or ResetTimer functions, because 
// they have global effect. 
//
// By default, RunParallel will spawn GOMAXPROCS goroutines,
// though SetParallelism may be called prior to RunParallel in order to 
// change this default. For CPU-bound benchmarks, increased
//  concurrency beyond GOMAXPROCS will have little effect on the 
// performance of the benchmark. If a benchmark already measures 
// concurrent performance (i.e. it spawns multiple goroutines),
// using RunParallel is not necessary.
A few thoughts:
* It might be worth just discussing this in a CL, if you'd like to start one.
* The old docs started with a short, concise sentence. It'd be nice to do the same here.
* I'm not sure that the increased verbiage pulls its weight.
* It appears that the main content change here is the last sentence. Based on our other
exchanges, you're trying to avoid confusion between benchmarking a function that uses
parallel computation under the hood and parallel benchmarking of a function (of any
flavor). I think that a nicely annotated example is a better way to achieve this goal.
And as written, the sentence is not accurate. Whether RunParallel is useful is
orthogonal to whether the body function itself starts new goroutines. Perhaps a better
way to convey this message is to emphasize that there's no point in using RunParallel if
there is no shared resource for which parallel execution of the body might contend.
* I suspect that some examples would help a lot. (Thanks, Dmitry, for the HTTP server
suggestion.)

[gopherbot]

Comment 5:

CL https://golang.org/cl/96910043 mentions this issue.

[robpike]

Comment 6:

Make a CL, but write something much shorter. This comment is too long. Plus I think the
original is fine.

[robpike]

Comment 7:

The existing comment is fine. It's very clear:
    RunParallel runs a benchmark in parallel. It creates multiple goroutines
    and distributes b.N iterations among them. The number of goroutines
    defaults to GOMAXPROCS.
If you don't know what that means, you shouldn't be using the function.

Status changed to WontFix.