benchseries: return error on time parse error
AllComparisonSeries shouldn't panic on bad input, it should return an
error.
For golang/go#53538.
Change-Id: I988b546ea3ad5bb9026956b3aa4feec809915b73
Reviewed-on: https://go-review.googlesource.com/c/perf/+/459177
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/benchseries/benchseries.go b/benchseries/benchseries.go
index 8b7f110..cdf1e30 100644
--- a/benchseries/benchseries.go
+++ b/benchseries/benchseries.go
@@ -197,17 +197,17 @@
// NormalizeDateString converts dates in two formats used in bent/benchmarking
// into UTC, so that all sort properly into a single order with no confusion.
-func NormalizeDateString(in string) string {
+func NormalizeDateString(in string) (string, error) {
if noPuncDate.MatchString(in) {
//20211229T213212
//2021-12-29T21:32:12
in = in[0:4] + "-" + in[4:6] + "-" + in[6:11] + ":" + in[11:13] + ":" + in[13:15] + "+00:00"
}
t, err := time.Parse(time.RFC3339Nano, in)
- if err == nil {
- return t.UTC().Format(RFC3339NanoNoZ)
+ if err != nil {
+ return "", err
}
- panic(err)
+ return t.UTC().Format(RFC3339NanoNoZ), nil
}
// ParseNormalizedDateString parses a time in the format returned by
@@ -430,7 +430,7 @@
// sensible order; this deals with that, including overlaps (depend on flag, either replaces old with
// younger or combines, REPLACE IS PREFERRED and works properly with combining old summary data with
// fresh benchmarking data) and possibly also with previously processed summaries.
-func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) []*ComparisonSeries {
+func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) ([]*ComparisonSeries, error) {
old := make(map[string]*ComparisonSeries)
for _, cs := range existing {
old[cs.Unit] = cs
@@ -484,13 +484,19 @@
for tk, tr := range t.cells {
// tk == bench, experiment, tr == baseline, tests, tests == map hash -> cell.
bench := tk.Benchmark
- dateString := NormalizeDateString(tk.Experiment.StringValues())
+ dateString, err := NormalizeDateString(tk.Experiment.StringValues())
+ if err != nil {
+ return nil, fmt.Errorf("error parsing experiment date %q: %w", tk.Experiment.StringValues(), err)
+ }
benchString := bench.StringValues()
benches[benchString] = struct{}{}
for hash, cell := range tr.tests {
hashString := hash.StringValues()
ser := b.hashToOrder[hash]
- serString := NormalizeDateString(ser.StringValues())
+ serString, err := NormalizeDateString(ser.StringValues())
+ if err != nil {
+ return nil, fmt.Errorf("error parsing series date %q: %w", ser.StringValues(), err)
+ }
sers[serString] = struct{}{}
sk := SeriesKey{
Benchmark: benchString,
@@ -637,7 +643,7 @@
}
}
- return css
+ return css, nil
}
func sortStringSet(m map[string]struct{}) []string {
diff --git a/cmd/benchseries/main.go b/cmd/benchseries/main.go
index 2337e50..dab8304 100644
--- a/cmd/benchseries/main.go
+++ b/cmd/benchseries/main.go
@@ -98,7 +98,10 @@
}
// Rearrange into comparisons (not yet doing the statistical work)
- comparisons = seriesBuilder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE)
+ comparisons, err = seriesBuilder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE)
+ if err != nil {
+ fail("Error building comparison series: %v", err)
+ }
// Chat about residues, per-table
for _, t := range comparisons {
diff --git a/cmd/benchseries/main_test.go b/cmd/benchseries/main_test.go
index 0319404..d390108 100644
--- a/cmd/benchseries/main_test.go
+++ b/cmd/benchseries/main_test.go
@@ -138,7 +138,10 @@
f.Close()
}
- comparisons = builder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE)
+ comparisons, err = builder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE)
+ if err != nil {
+ t.Fatalf("AllComparisonSeries(%+v) got err %v want nil", comparisons, err)
+ }
for _, c := range comparisons {
c.AddSummaries(0.95, 500) // 500 is faster than 1000.
