Skip to content

Commit 34caac8

Browse files
committed
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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent aa22272 commit 34caac8

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

benchseries/benchseries.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -925,9 +925,11 @@ func (cs *ComparisonSeries) AddSummaries(confidence float64, N int) {
925925
sum := c.Summary
926926
if sum == nil || (!sum.Present && sum.comparison == nil) {
927927
sum = &ComparisonSummary{comparison: c, Date: c.Date}
928-
sum.Center, sum.Low, sum.High = fn(c)
929-
sum.Present = true
930928
c.Summary = sum
929+
sum.Present = c.Denominator != nil
930+
if sum.Present {
931+
sum.Center, sum.Low, sum.High = fn(c)
932+
}
931933
}
932934
row = append(row, sum)
933935
} else {

benchseries/benchseries_test.go

+60
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,63 @@ func TestMissingCompare(t *testing.T) {
187187
t.Errorf("len(Series) got %d want 0; Series: %+v", len(got.Series), got.Series)
188188
}
189189
}
190+
191+
// A test point with no denominator.
192+
func TestMissingDenominator(t *testing.T) {
193+
results := []*benchfmt.Result{
194+
{
195+
Config: []benchfmt.Config{
196+
makeConfig("compare", "numerator"),
197+
makeConfig("runstamp", "2020-01-01T00:00:00Z"),
198+
makeConfig("numerator-hash", "abcdef0123456789"),
199+
makeConfig("denominator-hash", "9876543219fedcba"),
200+
makeConfig("numerator-hash-time", "2020-02-02T00:00:00Z"),
201+
makeConfig("residue", "foo"),
202+
},
203+
Name: []byte("Foo"),
204+
Iters: 10,
205+
Values: []benchfmt.Value{
206+
{Value: 10, Unit: "sec"},
207+
},
208+
},
209+
}
210+
211+
opts := &BuilderOptions{
212+
Filter: ".unit:/.*/",
213+
Series: "numerator-hash-time",
214+
Table: "", // .unit only
215+
Experiment: "runstamp",
216+
Compare: "compare",
217+
Numerator: "numerator",
218+
Denominator: "denominator",
219+
NumeratorHash: "numerator-hash",
220+
DenominatorHash: "denominator-hash",
221+
Warn: func(format string, args ...interface{}) {
222+
s := fmt.Sprintf(format, args...)
223+
t.Errorf("benchseries warning: %s", s)
224+
},
225+
}
226+
builder, err := NewBuilder(opts)
227+
if err != nil {
228+
t.Fatalf("NewBuilder get err %v, want err nil", err)
229+
}
230+
231+
for _, r := range results {
232+
builder.Add(r)
233+
}
234+
235+
comparisons, err := builder.AllComparisonSeries(nil, DUPE_REPLACE)
236+
if err != nil {
237+
t.Fatalf("AllComparisonSeries got err %v want err nil", err)
238+
}
239+
240+
if len(comparisons) != 1 {
241+
t.Fatalf("len(comparisons) got %d want 1; comparisons: %+v", len(comparisons), comparisons)
242+
}
243+
244+
got := comparisons[0]
245+
got.AddSummaries(0.95, 1000)
246+
if len(got.Summaries) != 1 {
247+
t.Fatalf("Expect 1 comparison, got: %+v", len(got.Summaries))
248+
}
249+
}

0 commit comments

Comments
 (0)