benchseries: do not crash if no denominator is present
On ppc64le, I am running a slightly modified instance of x/build perf
to monitor performance between the latest release toolchain and
upstream.
I started to glob run the crypto benchmarks this release, and
unsuprisingly some were added during development. This caused a
failure to ingest new results when syncing with influx.
fixes golang/go#66685
Change-Id: Id5145b779f48afa2797a861d047d9d46a263b229
Reviewed-on: https://go-review.googlesource.com/c/perf/+/576695
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
diff --git a/benchseries/benchseries.go b/benchseries/benchseries.go
index d40c827..d284a91 100644
--- a/benchseries/benchseries.go
+++ b/benchseries/benchseries.go
@@ -925,9 +925,11 @@
sum := c.Summary
if sum == nil || (!sum.Present && sum.comparison == nil) {
sum = &ComparisonSummary{comparison: c, Date: c.Date}
- sum.Center, sum.Low, sum.High = fn(c)
- sum.Present = true
c.Summary = sum
+ sum.Present = c.Denominator != nil
+ if sum.Present {
+ sum.Center, sum.Low, sum.High = fn(c)
+ }
}
row = append(row, sum)
} else {
diff --git a/benchseries/benchseries_test.go b/benchseries/benchseries_test.go
index d65b53f..369094a 100644
--- a/benchseries/benchseries_test.go
+++ b/benchseries/benchseries_test.go
@@ -187,3 +187,63 @@
t.Errorf("len(Series) got %d want 0; Series: %+v", len(got.Series), got.Series)
}
}
+
+// A test point with no denominator.
+func TestMissingDenominator(t *testing.T) {
+ results := []*benchfmt.Result{
+ {
+ Config: []benchfmt.Config{
+ makeConfig("compare", "numerator"),
+ makeConfig("runstamp", "2020-01-01T00:00:00Z"),
+ makeConfig("numerator-hash", "abcdef0123456789"),
+ makeConfig("denominator-hash", "9876543219fedcba"),
+ makeConfig("numerator-hash-time", "2020-02-02T00:00:00Z"),
+ makeConfig("residue", "foo"),
+ },
+ Name: []byte("Foo"),
+ Iters: 10,
+ Values: []benchfmt.Value{
+ {Value: 10, Unit: "sec"},
+ },
+ },
+ }
+
+ opts := &BuilderOptions{
+ Filter: ".unit:/.*/",
+ Series: "numerator-hash-time",
+ Table: "", // .unit only
+ Experiment: "runstamp",
+ Compare: "compare",
+ Numerator: "numerator",
+ Denominator: "denominator",
+ NumeratorHash: "numerator-hash",
+ DenominatorHash: "denominator-hash",
+ Warn: func(format string, args ...interface{}) {
+ s := fmt.Sprintf(format, args...)
+ t.Errorf("benchseries warning: %s", s)
+ },
+ }
+ builder, err := NewBuilder(opts)
+ if err != nil {
+ t.Fatalf("NewBuilder get err %v, want err nil", err)
+ }
+
+ for _, r := range results {
+ builder.Add(r)
+ }
+
+ comparisons, err := builder.AllComparisonSeries(nil, DUPE_REPLACE)
+ if err != nil {
+ t.Fatalf("AllComparisonSeries got err %v want err nil", err)
+ }
+
+ if len(comparisons) != 1 {
+ t.Fatalf("len(comparisons) got %d want 1; comparisons: %+v", len(comparisons), comparisons)
+ }
+
+ got := comparisons[0]
+ got.AddSummaries(0.95, 1000)
+ if len(got.Summaries) != 1 {
+ t.Fatalf("Expect 1 comparison, got: %+v", len(got.Summaries))
+ }
+}
