Skip to content

Commit

Permalink
benchseries: do not crash if no denominator is present
Browse files Browse the repository at this point in the history
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 <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
  • Loading branch information
pmur committed Nov 18, 2024
1 parent aa22272 commit 34caac8
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 2 deletions.
6 changes: 4 additions & 2 deletions benchseries/benchseries.go
Original file line number Diff line number Diff line change
Expand Up @@ -925,9 +925,11 @@ func (cs *ComparisonSeries) AddSummaries(confidence float64, N int) {
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 {
Expand Down
60 changes: 60 additions & 0 deletions benchseries/benchseries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,63 @@ func TestMissingCompare(t *testing.T) {
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))
}
}

0 comments on commit 34caac8

Please sign in to comment.