Skip to content

Commit

Permalink
benchseries: return error on time parse error
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
prattmic authored and gopherbot committed Dec 22, 2022
1 parent e8d778a commit 3fd27c2
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 10 deletions.
22 changes: 14 additions & 8 deletions benchseries/benchseries.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,17 @@ const RFC3339NanoNoZ = "2006-01-02T15:04:05.999999999-07:00"

// 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
Expand Down Expand Up @@ -430,7 +430,7 @@ const (
// 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
Expand Down Expand Up @@ -484,13 +484,19 @@ func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int)
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,
Expand Down Expand Up @@ -637,7 +643,7 @@ func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int)
}
}

return css
return css, nil
}

func sortStringSet(m map[string]struct{}) []string {
Expand Down
5 changes: 4 additions & 1 deletion cmd/benchseries/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ func main() {
}

// 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 {
Expand Down
5 changes: 4 additions & 1 deletion cmd/benchseries/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ func combineFilesAndJson(t *testing.T, files []string, jsonFile string) []*bench
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.
Expand Down

0 comments on commit 3fd27c2

Please sign in to comment.