benchmark/parse, cmd/benchcmp: new package for parsing benchmarks

Move the parser for benchmark output from cmd/benchcmp into its own
package, benchmark/parse.

The majority of the change is just moving code around. Instead of
implementing the '-best' flag in ParseBenchSet, it is now implemented in
its own function 'selectBest' in cmd/benchcmp.

Bench.Ord (the ordinal position of a Bench within a BenchSet) has been
exported.

Change-Id: Id27032a220f9ff2596117b58b86243998695a804
Reviewed-on: https://go-review.googlesource.com/2102
Reviewed-by: Rob Pike <r@golang.org>

6 files changed, 124 insertions, 84 deletions

diff --git a/cmd/benchcmp/parse.go b/benchmark/parse/parse.go
index f1df681..6464dea 100644
--- a/cmd/benchcmp/parse.go
+++ b/benchmark/parse/parse.go
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package main
+// Package parse provides support for parsing benchmark results as
+// generated by 'go test -bench'.
+package parse // import "golang.org/x/tools/benchmark/parse"
 
 import (
 	"bufio"
@@ -31,7 +33,7 @@ type Bench struct {
 	BOp      uint64  // bytes allocated per iteration
 	AllocsOp uint64  // allocs per iteration
 	Measured int     // which measurements were recorded
-	ord      int     // ordinal position within a benchmark run, used for sorting
+	Ord      int     // ordinal position within a benchmark run
 }
 
 // ParseLine extracts a Bench from a single line of testing.B output.
@@ -105,24 +107,17 @@ func (b *Bench) String() string {
 // testing.B run, keyed by name to facilitate comparison.
 type BenchSet map[string][]*Bench
 
-// Parse extracts a BenchSet from testing.B output. Parse
-// preserves the order of benchmarks that have identical names.
+// ParseBenchSet extracts a BenchSet from testing.B output.
+// ParseBenchSet preserves the order of benchmarks that have identical
+// names.
 func ParseBenchSet(r io.Reader) (BenchSet, error) {
 	bb := make(BenchSet)
 	scan := bufio.NewScanner(r)
 	ord := 0
 	for scan.Scan() {
 		if b, err := ParseLine(scan.Text()); err == nil {
-			b.ord = ord
+			b.Ord = ord
 			ord++
-			old := bb[b.Name]
-			if *best && old != nil {
-				if old[0].NsOp < b.NsOp {
-					continue
-				}
-				b.ord = old[0].ord
-				bb[b.Name] = old[:0]
-			}
 			bb[b.Name] = append(bb[b.Name], b)
 		}
 	}
diff --git a/cmd/benchcmp/parse_test.go b/benchmark/parse/parse_test.go
index a59b20c..d96be63 100644
--- a/cmd/benchcmp/parse_test.go
+++ b/benchmark/parse/parse_test.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package main
+package parse
 
 import (
 	"reflect"
@@ -117,7 +117,7 @@ func TestParseBenchSet(t *testing.T) {
 				Name: "BenchmarkReadRequestApachebench",
 				N:    1000000, NsOp: 2960, MbS: 27.70, BOp: 839, AllocsOp: 9,
 				Measured: NsOp | MbS | BOp | AllocsOp,
-				ord:      2,
+				Ord:      2,
 			},
 		},
 		"BenchmarkClientServerParallel64": []*Bench{
@@ -125,7 +125,7 @@ func TestParseBenchSet(t *testing.T) {
 				Name: "BenchmarkClientServerParallel64",
 				N:    50000, NsOp: 59192, BOp: 7028, AllocsOp: 60,
 				Measured: NsOp | BOp | AllocsOp,
-				ord:      3,
+				Ord:      3,
 			},
 		},
 		"BenchmarkEncrypt": []*Bench{
@@ -133,53 +133,13 @@ func TestParseBenchSet(t *testing.T) {
 				Name: "BenchmarkEncrypt",
 				N:    100000000, NsOp: 19.6,
 				Measured: NsOp,
-				ord:      0,
+				Ord:      0,
 			},
 			{
 				Name: "BenchmarkEncrypt",
 				N:    5000000, NsOp: 517,
 				Measured: NsOp,
-				ord:      1,
-			},
-		},
-	}
-
-	have, err := ParseBenchSet(strings.NewReader(in))
-	if err != nil {
-		t.Fatalf("unexpected err during ParseBenchSet: %v", err)
-	}
-	if !reflect.DeepEqual(want, have) {
-		t.Errorf("parsed bench set incorrectly, want %v have %v", want, have)
-	}
-}
-
-func TestParseBenchSetBest(t *testing.T) {
-	// Test that -best mode takes best ns/op.
-	*best = true
-	defer func() {
-		*best = false
-	}()
-
-	in := `
-		Benchmark1 10 100 ns/op
-		Benchmark2 10 60 ns/op
-		Benchmark2 10 500 ns/op
-		Benchmark1 10 50 ns/op
-	`
-
-	want := BenchSet{
-		"Benchmark1": []*Bench{
-			{
-				Name: "Benchmark1",
-				N:    10, NsOp: 50, Measured: NsOp,
-				ord: 0,
-			},
-		},
-		"Benchmark2": []*Bench{
-			{
-				Name: "Benchmark2",
-				N:    10, NsOp: 60, Measured: NsOp,
-				ord: 1,
+				Ord:      1,
 			},
 		},
 	}
diff --git a/cmd/benchcmp/benchcmp.go b/cmd/benchcmp/benchcmp.go
index 9bfe2e3..e32c9c3 100644
--- a/cmd/benchcmp/benchcmp.go
+++ b/cmd/benchcmp/benchcmp.go
@@ -11,6 +11,8 @@ import (
 	"sort"
 	"strconv"
 	"text/tabwriter"
+
+	"golang.org/x/tools/benchmark/parse"
 )
 
 var (
@@ -66,7 +68,7 @@ func main() {
 		sort.Sort(ByParseOrder(cmps))
 	}
 	for _, cmp := range cmps {
-		if !cmp.Measured(NsOp) {
+		if !cmp.Measured(parse.NsOp) {
 			continue
 		}
 		if delta := cmp.DeltaNsOp(); !*changedOnly || delta.Changed() {
@@ -83,7 +85,7 @@ func main() {
 		sort.Sort(ByDeltaMbS(cmps))
 	}
 	for _, cmp := range cmps {
-		if !cmp.Measured(MbS) {
+		if !cmp.Measured(parse.MbS) {
 			continue
 		}
 		if delta := cmp.DeltaMbS(); !*changedOnly || delta.Changed() {
@@ -100,7 +102,7 @@ func main() {
 		sort.Sort(ByDeltaAllocsOp(cmps))
 	}
 	for _, cmp := range cmps {
-		if !cmp.Measured(AllocsOp) {
+		if !cmp.Measured(parse.AllocsOp) {
 			continue
 		}
 		if delta := cmp.DeltaAllocsOp(); !*changedOnly || delta.Changed() {
@@ -117,7 +119,7 @@ func main() {
 		sort.Sort(ByDeltaBOp(cmps))
 	}
 	for _, cmp := range cmps {
-		if !cmp.Measured(BOp) {
+		if !cmp.Measured(parse.BOp) {
 			continue
 		}
 		if delta := cmp.DeltaBOp(); !*changedOnly || delta.Changed() {
@@ -135,18 +137,38 @@ func fatal(msg interface{}) {
 	os.Exit(1)
 }
 
-func parseFile(path string) BenchSet {
+func parseFile(path string) parse.BenchSet {
 	f, err := os.Open(path)
 	if err != nil {
 		fatal(err)
 	}
-	bb, err := ParseBenchSet(f)
+	bb, err := parse.ParseBenchSet(f)
 	if err != nil {
 		fatal(err)
 	}
+	if *best {
+		selectBest(bb)
+	}
 	return bb
 }
 
+func selectBest(bs parse.BenchSet) {
+	for name, bb := range bs {
+		if len(bb) < 2 {
+			continue
+		}
+		ord := bb[0].Ord
+		best := bb[0]
+		for _, b := range bb {
+			if b.NsOp < best.NsOp {
+				b.Ord = ord
+				best = b
+			}
+		}
+		bs[name] = []*parse.Bench{best}
+	}
+}
+
 // formatNs formats ns measurements to expose a useful amount of
 // precision. It mirrors the ns precision logic of testing.B.
 func formatNs(ns float64) string {
diff --git a/cmd/benchcmp/benchcmp_test.go b/cmd/benchcmp/benchcmp_test.go
new file mode 100644
index 0000000..f57e58c
--- /dev/null
+++ b/cmd/benchcmp/benchcmp_test.go
@@ -0,0 +1,59 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+
+	"golang.org/x/tools/benchmark/parse"
+)
+
+func TestSelectBest(t *testing.T) {
+	have := parse.BenchSet{
+		"Benchmark1": []*parse.Bench{
+			{
+				Name: "Benchmark1",
+				N:    10, NsOp: 100, Measured: parse.NsOp,
+				Ord: 0,
+			},
+			{
+				Name: "Benchmark1",
+				N:    10, NsOp: 50, Measured: parse.NsOp,
+				Ord: 3,
+			},
+		},
+		"Benchmark2": []*parse.Bench{
+			{
+				Name: "Benchmark2",
+				N:    10, NsOp: 60, Measured: parse.NsOp,
+				Ord: 1,
+			},
+			{
+				Name: "Benchmark2",
+				N:    10, NsOp: 500, Measured: parse.NsOp,
+				Ord: 2,
+			},
+		},
+	}
+
+	want := parse.BenchSet{
+		"Benchmark1": []*parse.Bench{
+			{
+				Name: "Benchmark1",
+				N:    10, NsOp: 50, Measured: parse.NsOp,
+				Ord: 0,
+			},
+		},
+		"Benchmark2": []*parse.Bench{
+			{
+				Name: "Benchmark2",
+				N:    10, NsOp: 60, Measured: parse.NsOp,
+				Ord: 1,
+			},
+		},
+	}
+
+	selectBest(have)
+	if !reflect.DeepEqual(want, have) {
+		t.Errorf("filtered bench set incorrectly, want %v have %v", want, have)
+	}
+}
diff --git a/cmd/benchcmp/compare.go b/cmd/benchcmp/compare.go
index 9ebe426..921b827 100644
--- a/cmd/benchcmp/compare.go
+++ b/cmd/benchcmp/compare.go
@@ -7,16 +7,18 @@ package main
 import (
 	"fmt"
 	"math"
+
+	"golang.org/x/tools/benchmark/parse"
 )
 
 // BenchCmp is a pair of benchmarks.
 type BenchCmp struct {
-	Before *Bench
-	After  *Bench
+	Before *parse.Bench
+	After  *parse.Bench
 }
 
 // Correlate correlates benchmarks from two BenchSets.
-func Correlate(before, after BenchSet) (cmps []BenchCmp, warnings []string) {
+func Correlate(before, after parse.BenchSet) (cmps []BenchCmp, warnings []string) {
 	cmps = make([]BenchCmp, 0, len(after))
 	for name, beforebb := range before {
 		afterbb := after[name]
@@ -102,7 +104,7 @@ type ByParseOrder []BenchCmp
 
 func (x ByParseOrder) Len() int           { return len(x) }
 func (x ByParseOrder) Swap(i, j int)      { x[i], x[j] = x[j], x[i] }
-func (x ByParseOrder) Less(i, j int) bool { return x[i].Before.ord < x[j].Before.ord }
+func (x ByParseOrder) Less(i, j int) bool { return x[i].Before.Ord < x[j].Before.Ord }
 
 // lessByDelta provides lexicographic ordering:
 //   * largest delta by magnitude
diff --git a/cmd/benchcmp/compare_test.go b/cmd/benchcmp/compare_test.go
index 5baca66..73b3cce 100644
--- a/cmd/benchcmp/compare_test.go
+++ b/cmd/benchcmp/compare_test.go
@@ -9,6 +9,8 @@ import (
 	"reflect"
 	"sort"
 	"testing"
+
+	"golang.org/x/tools/benchmark/parse"
 )
 
 func TestDelta(t *testing.T) {
@@ -52,29 +54,29 @@ func TestCorrelate(t *testing.T) {
 	// Benches that are going to be successfully correlated get N thus:
 	//   0x<counter><num benches><b = before | a = after>
 	// Read this: "<counter> of <num benches>, from <before|after>".
-	before := BenchSet{
-		"BenchmarkOneEach":   []*Bench{{Name: "BenchmarkOneEach", N: 0x11b}},
-		"BenchmarkOneToNone": []*Bench{{Name: "BenchmarkOneToNone"}},
-		"BenchmarkOneToTwo":  []*Bench{{Name: "BenchmarkOneToTwo"}},
-		"BenchmarkTwoToOne": []*Bench{
+	before := parse.BenchSet{
+		"BenchmarkOneEach":   []*parse.Bench{{Name: "BenchmarkOneEach", N: 0x11b}},
+		"BenchmarkOneToNone": []*parse.Bench{{Name: "BenchmarkOneToNone"}},
+		"BenchmarkOneToTwo":  []*parse.Bench{{Name: "BenchmarkOneToTwo"}},
+		"BenchmarkTwoToOne": []*parse.Bench{
 			{Name: "BenchmarkTwoToOne"},
 			{Name: "BenchmarkTwoToOne"},
 		},
-		"BenchmarkTwoEach": []*Bench{
+		"BenchmarkTwoEach": []*parse.Bench{
 			{Name: "BenchmarkTwoEach", N: 0x12b},
 			{Name: "BenchmarkTwoEach", N: 0x22b},
 		},
 	}
 
-	after := BenchSet{
-		"BenchmarkOneEach":   []*Bench{{Name: "BenchmarkOneEach", N: 0x11a}},
-		"BenchmarkNoneToOne": []*Bench{{Name: "BenchmarkNoneToOne"}},
-		"BenchmarkTwoToOne":  []*Bench{{Name: "BenchmarkTwoToOne"}},
-		"BenchmarkOneToTwo": []*Bench{
+	after := parse.BenchSet{
+		"BenchmarkOneEach":   []*parse.Bench{{Name: "BenchmarkOneEach", N: 0x11a}},
+		"BenchmarkNoneToOne": []*parse.Bench{{Name: "BenchmarkNoneToOne"}},
+		"BenchmarkTwoToOne":  []*parse.Bench{{Name: "BenchmarkTwoToOne"}},
+		"BenchmarkOneToTwo": []*parse.Bench{
 			{Name: "BenchmarkOneToTwo"},
 			{Name: "BenchmarkOneToTwo"},
 		},
-		"BenchmarkTwoEach": []*Bench{
+		"BenchmarkTwoEach": []*parse.Bench{
 			{Name: "BenchmarkTwoEach", N: 0x12a},
 			{Name: "BenchmarkTwoEach", N: 0x22a},
 		},
@@ -108,10 +110,10 @@ func TestCorrelate(t *testing.T) {
 
 func TestBenchCmpSorting(t *testing.T) {
 	c := []BenchCmp{
-		{&Bench{Name: "BenchmarkMuchFaster", NsOp: 10, ord: 3}, &Bench{Name: "BenchmarkMuchFaster", NsOp: 1}},
-		{&Bench{Name: "BenchmarkSameB", NsOp: 5, ord: 1}, &Bench{Name: "BenchmarkSameB", NsOp: 5}},
-		{&Bench{Name: "BenchmarkSameA", NsOp: 5, ord: 2}, &Bench{Name: "BenchmarkSameA", NsOp: 5}},
-		{&Bench{Name: "BenchmarkSlower", NsOp: 10, ord: 0}, &Bench{Name: "BenchmarkSlower", NsOp: 11}},
+		{&parse.Bench{Name: "BenchmarkMuchFaster", NsOp: 10, Ord: 3}, &parse.Bench{Name: "BenchmarkMuchFaster", NsOp: 1}},
+		{&parse.Bench{Name: "BenchmarkSameB", NsOp: 5, Ord: 1}, &parse.Bench{Name: "BenchmarkSameB", NsOp: 5}},
+		{&parse.Bench{Name: "BenchmarkSameA", NsOp: 5, Ord: 2}, &parse.Bench{Name: "BenchmarkSameA", NsOp: 5}},
+		{&parse.Bench{Name: "BenchmarkSlower", NsOp: 10, Ord: 0}, &parse.Bench{Name: "BenchmarkSlower", NsOp: 11}},
 	}
 
 	// Test just one magnitude-based sort order; they are symmetric.