storage: coalesce results with the same labels into one row
Change-Id: Ia9d62651ba23794cd64dfcf2ea2e529914f3b100
Reviewed-on: https://go-review.googlesource.com/35674
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/storage/benchfmt/benchfmt.go b/storage/benchfmt/benchfmt.go
index 37f33b7..e43e48d 100644
--- a/storage/benchfmt/benchfmt.go
+++ b/storage/benchfmt/benchfmt.go
@@ -9,6 +9,7 @@
import (
"bufio"
+ "bytes"
"fmt"
"io"
"sort"
@@ -53,7 +54,7 @@
// AddLabels adds additional labels as if they had been read from the header of a file.
// It must be called before the first call to r.Next.
func (r *Reader) AddLabels(labels Labels) {
- r.permLabels = labels.copy()
+ r.permLabels = labels.Copy()
for k, v := range labels {
r.labels[k] = v
}
@@ -61,6 +62,7 @@
// Result represents a single line from a benchmark file.
// All information about that line is self-contained in the Result.
+// A Result is immutable once created.
type Result struct {
// Labels is the set of persistent labels that apply to the result.
// Labels must not be modified.
@@ -75,10 +77,29 @@
Content string
}
+// SameLabels reports whether r and b have the same labels.
+func (r *Result) SameLabels(b *Result) bool {
+ return r.Labels.Equal(b.Labels) && r.NameLabels.Equal(b.NameLabels)
+}
+
// Labels is a set of key-value strings.
type Labels map[string]string
-// TODO(quentin): Add String and Equal methods to Labels?
+// String returns the labels formatted as a comma-separated
+// list enclosed in braces.
+func (l Labels) String() string {
+ var out bytes.Buffer
+ out.WriteString("{")
+ for k, v := range l {
+ fmt.Fprintf(&out, "%q: %q, ", k, v)
+ }
+ if out.Len() > 1 {
+ // Remove extra ", "
+ out.Truncate(out.Len() - 2)
+ }
+ out.WriteString("}")
+ return out.String()
+}
// Keys returns a sorted list of the keys in l.
func (l Labels) Keys() []string {
@@ -90,6 +111,19 @@
return out
}
+// Equal reports whether l and b have the same keys and values.
+func (l Labels) Equal(b Labels) bool {
+ if len(l) != len(b) {
+ return false
+ }
+ for k := range l {
+ if l[k] != b[k] {
+ return false
+ }
+ }
+ return true
+}
+
// A Printer prints a sequence of benchmark results.
type Printer struct {
w io.Writer
@@ -178,9 +212,9 @@
return res
}
-// copy returns a new copy of the labels map, to protect against
+// Copy returns a new copy of the labels map, to protect against
// future modifications to labels.
-func (l Labels) copy() Labels {
+func (l Labels) Copy() Labels {
new := make(Labels)
for k, v := range l {
new[k] = v
@@ -207,7 +241,7 @@
}
if !copied {
copied = true
- r.labels = r.labels.copy()
+ r.labels = r.labels.Copy()
}
// TODO(quentin): Spec says empty value is valid, but
// we need a way to cancel previous labels, so we'll
@@ -222,7 +256,7 @@
// Blank line delimits the header. If we find anything else, the file must not have a header.
if !havePerm {
if line == "" {
- r.permLabels = r.labels.copy()
+ r.permLabels = r.labels.Copy()
} else {
r.permLabels = Labels{}
}
diff --git a/storage/db/db.go b/storage/db/db.go
index decb63a..35b696f 100644
--- a/storage/db/db.go
+++ b/storage/db/db.go
@@ -167,6 +167,7 @@
// pending arguments for flush
insertRecordArgs []interface{}
insertLabelArgs []interface{}
+ lastResult *benchfmt.Result
}
// now is a hook for testing
@@ -267,11 +268,19 @@
// InsertRecord inserts a single record in an existing upload.
// If InsertRecord returns a non-nil error, the Upload has failed and u.Abort() must be called.
func (u *Upload) InsertRecord(r *benchfmt.Result) error {
+ if u.lastResult != nil && u.lastResult.SameLabels(r) {
+ data := u.insertRecordArgs[len(u.insertRecordArgs)-1].([]byte)
+ data = append(data, r.Content...)
+ data = append(data, '\n')
+ u.insertRecordArgs[len(u.insertRecordArgs)-1] = data
+ return nil
+ }
// TODO(quentin): Support multiple lines (slice of results?)
var buf bytes.Buffer
if err := benchfmt.NewPrinter(&buf).Print(r); err != nil {
return err
}
+ u.lastResult = r
u.insertRecordArgs = append(u.insertRecordArgs, u.ID, u.recordid, buf.Bytes())
for _, k := range r.Labels.Keys() {
if err := u.insertLabel(k, r.Labels[k]); err != nil {
@@ -331,6 +340,7 @@
}
u.insertLabelArgs = nil
}
+ u.lastResult = nil
return nil
}
diff --git a/storage/db/db_test.go b/storage/db/db_test.go
index 9b108eb..154f4b1 100644
--- a/storage/db/db_test.go
+++ b/storage/db/db_test.go
@@ -114,24 +114,25 @@
ctx := context.Background()
- r := &benchfmt.Result{
- benchfmt.Labels{"key": "value"},
- nil,
- 1,
- "BenchmarkName 1 ns/op",
- }
+ labels := benchfmt.Labels{"key": "value"}
+
u, err := db.NewUpload(ctx)
if err != nil {
t.Fatalf("NewUpload: %v", err)
}
- r.Labels["uploadid"] = u.ID
+ labels["uploadid"] = u.ID
for _, num := range []string{"1", "2"} {
- r.Labels["num"] = num
+ labels["num"] = num
for _, num2 := range []int{1, 2} {
- r.Content = fmt.Sprintf("BenchmarkName %d ns/op", num2)
- if err := u.InsertRecord(r); err != nil {
+ if err := u.InsertRecord(&benchfmt.Result{
+ labels,
+ nil,
+ 1,
+ fmt.Sprintf("BenchmarkName %d ns/op", num2),
+ }); err != nil {
t.Fatalf("InsertRecord: %v", err)
}
+ labels = labels.Copy()
}
}
@@ -150,18 +151,23 @@
BenchmarkName 2 ns/op
`)
- r.Labels["num"] = "3"
- r.Content = "BenchmarkName 3 ns/op"
+ labels["num"] = "3"
for _, uploadid := range []string{u.ID, "new"} {
u, err := db.ReplaceUpload(uploadid)
if err != nil {
t.Fatalf("ReplaceUpload: %v", err)
}
- r.Labels["uploadid"] = u.ID
- if err := u.InsertRecord(r); err != nil {
+ labels["uploadid"] = u.ID
+ if err := u.InsertRecord(&benchfmt.Result{
+ labels,
+ nil,
+ 1,
+ "BenchmarkName 3 ns/op",
+ }); err != nil {
t.Fatalf("InsertRecord: %v", err)
}
+ labels = labels.Copy()
if err := u.Commit(); err != nil {
t.Fatalf("Commit: %v", err)
@@ -193,15 +199,16 @@
br := benchfmt.NewReader(strings.NewReader(`
key: value
BenchmarkName 1 ns/op
+BenchmarkName 2 ns/op
`))
- if !br.Next() {
- t.Fatalf("unable to read test string: %v", br.Err())
+ for br.Next() {
+ if err := u.InsertRecord(br.Result()); err != nil {
+ t.Fatalf("InsertRecord: %v", err)
+ }
}
-
- if err := u.InsertRecord(br.Result()); err != nil {
- t.Fatalf("InsertRecord: %v", err)
+ if err := br.Err(); err != nil {
+ t.Fatalf("Err: %v", err)
}
-
if err := u.Commit(); err != nil {
t.Fatalf("Commit: %v", err)
}
