[perf] cmd/benchstat: new version of benchstat
Austin Clements (Gerrit)
Attention is currently required from: David Chase, Russ Cox.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: David Chase, Russ Cox.
Austin Clements uploaded patch set #3 to this change.
cmd/benchstat: new version of benchstat
This is a complete rewrite of benchstat. Basic usage remains the same,
as does the core idea of showing statistical benchmark summaries and
A/B comparisons in a table, but there are several major improvements.
The statistics is now more robust. Previously, benchstat used
IQR-based outlier rejection, showed the mean of the reduced sample,
its range, and did a non-parametric difference-of-distribution test on
reduced samples. Any form of outlier rejection must start with
distributional assumptions, in this case assuming normality, which is
generally not sound for benchmark data. Hence, now benchstat does not
do any outlier rejection. As a result, it must use robust summary
statistics as well, so benchstat now uses the median and confidence
interval of the median as summary statistics. Benchstat continues to
use the same Mann-Whitney U-test for the delta, but now does it on the
full samples since the U-test is already non-parametric, hence
increasing the power of this test.
As part of these statistical improvements, benchstat now detects and
warns about several common mistakes, such as having too few samples
for meaningful statistical results, or having incomparable geomeans.
The output format is more consistent. Previously, benchstat
transformed units like "ns/op" into a metric like "time/op", which it
used as a column header; and a numerator like "sec", which it used to
label each measurement. This was easy enough for the standard units
used by the testing framework, but was basically impossible to
generalize to custom units. Now, benchstat does unit scaling, but
otherwise leaves units alone. The full (scaled) unit is used as a
column header and each measurement is simply a scaled value shown with
an SI prefix. This also means that the text and CSV formats can be
much more similar while still allowing the CSV format to be usefully
machine-readable.
Benchstat will also now do A/B comparisons even if there are more than
two inputs. It shows a comparison to the base in the second and all
subsequent columns. This approach is consistent for any number of
inputs.
Benchstat now supports the full Go benchmark format, including
sophisticated control over exactly how it structures the results into
rows, columns, and tables. This makes it easy to do meaningful
comparisons across benchmark data that isn't simply structured into
two input files, and gives significantly more control over how results
are sorted. The default behavior is still to turn each input file into
a column and each benchmark into a row.
Fixes golang/go#19565 by showing all results, even if the benchmark
sets don't match across columns, and warning when geomean sets are
incompatible.
Fixes golang/go#19634 by no longer doing outlier rejection and clearly
reporting when there are not enough samples to do a meaningful
difference test.
Updates golang/go#23471 by providing more through command
documentation. I'm not sure it quite fixes this issue, but it's much
better than it was.
Fixes golang/go#30368 because benchstat now supports filter
expressions, which can also filter down units.
Fixes golang/go#33169 because benchstat now always shows file
configuration labels.
Updates golang/go#43744 by integrating unit metadata to control
statistical assumptions into the main tool that implements those
assumptions.
Fixes golang/go#48380 by introducing a way to override labels from the
command line rather than always using file names.
Change-Id: Ie2c5a12024e84b4918e483df2223eb1f10413a4f
---
D cmd/benchstat/testdata/allnosplitcsv.golden
D cmd/benchstat/testdata/packagesold.txt
D cmd/benchstat/testdata/packagesold.golden
D cmd/benchstat/testdata/slashslash4.txt
A cmd/benchstat/testdata/crcOldNew.stdout
D cmd/benchstat/testdata/rdeltasort.golden
A cmd/benchstat/testdata/issue19634.stdout
D cmd/benchstat/testdata/units-new.txt
D cmd/benchstat/testdata/examplehtml.golden
A cmd/benchstat/testdata/docColFormat.stdout
D cmd/benchstat/testdata/namesort.golden
A cmd/benchstat/internal/benchtab/table.go
D cmd/benchstat/testdata/allnosplit.golden
A cmd/benchstat/testdata/smallSample.txt
A cmd/benchstat/testdata/units.txt
D cmd/benchstat/testdata/new4.golden
D cmd/benchstat/testdata/zero-old.txt
A cmd/benchstat/testdata/issue19634.txt
A cmd/benchstat/testdata/docSorting.stdout
A cmd/benchstat/testdata/units.stdout
D cmd/benchstat/testdata/all.golden
D cmd/benchstat/testdata/units.golden
D cmd/benchstat/testdata/exampleold.txt
A cmd/benchstat/testdata/crcSizeVsPoly.stdout
A cmd/benchstat/testdata/smallSample.stdout
A cmd/benchstat/internal/texttab/table.go
A cmd/benchstat/testdata/docLabels.stdout
A cmd/benchstat/testdata/issue19565.stdout
M cmd/benchstat/testdata/new.txt
D cmd/benchstat/testdata/allnorangecsv.golden
A cmd/benchstat/internal/benchtab/builder.go
A cmd/benchstat/testdata/crc-new.txt
A cmd/benchstat/testdata/issue19565.txt
D cmd/benchstat/testdata/packagesnew.txt
D cmd/benchstat/testdata/oldnewttest.golden
A cmd/benchstat/testdata/zero.stdout
D cmd/benchstat/testdata/oldnew4html.golden
D cmd/benchstat/testdata/zero.golden
A cmd/benchstat/testdata/.gitignore
A cmd/benchstat/testdata/csvErrors.stderr
A cmd/benchstat/testdata/csvOldNew.stdout
D cmd/benchstat/testdata/oldnewgeo.golden
D cmd/benchstat/testdata/examplenew.txt
D cmd/benchstat/testdata/oldcsv.golden
D cmd/benchstat/testdata/exampleoldhtml.golden
D cmd/benchstat/testdata/units-old.txt
D cmd/benchstat/testdata/deltasort.golden
D cmd/benchstat/testdata/packages.golden
A cmd/benchstat/internal/texttab/table_test.go
A cmd/benchstat/testdata/csvErrors.stdout
M cmd/benchstat/main_test.go
A cmd/benchstat/testdata/docColFormat2.stdout
M cmd/benchstat/testdata/old.txt
D cmd/benchstat/testdata/exampleold.golden
A cmd/benchstat/testdata/zero.txt
D cmd/benchstat/testdata/oldnew.golden
A benchstat/doc.go
D cmd/benchstat/testdata/allcsv.golden
A cmd/benchstat/testdata/docOldNew.stdout
D cmd/benchstat/testdata/oldnewhtml.golden
D cmd/benchstat/testdata/zero-new.txt
A cmd/benchstat/testdata/bench_test.go
D cmd/benchstat/testdata/x386.txt
A cmd/benchstat/testdata/crcIgnore.stdout
M cmd/benchstat/main.go
A cmd/benchstat/testdata/crc-old.txt
D cmd/benchstat/README.md
A cmd/benchstat/testdata/docRowName.stdout
D cmd/benchstat/testdata/example.golden
69 files changed, 3,127 insertions(+), 3,156 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Dan Kortschak (Gerrit)
Attention is currently required from: Austin Clements, David Chase, Russ Cox.
3 comments:
Patchset:
Nice work.
File cmd/benchstat/main.go:
Patch Set #3, Line 104:
confidence interval summariesSpecify what the CI is? 95%?
// Finally, the last row of the table shows the geometric mean of each
// column, giving an overall picture of how the benchmarks moved.Given that benchmarks in a suite may operate on wildly different timescales, this seems like an unsound thing to do.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: David Chase, Dan Kortschak, Russ Cox.
3 comments:
Patchset:
Nice work.
Thanks! This has been a long time in the making.
File cmd/benchstat/main.go:
Patch Set #3, Line 104:
confidence interval summariesSpecify what the CI is? 95%?
Done (this is configurable via a flag, but does default to 95%)
// Finally, the last row of the table shows the geometric mean of each
// column, giving an overall picture of how the benchmarks moved.
Given that benchmarks in a suite may operate on wildly different timescales, this seems like an unso […]
This is why we use a geometric mean instead of an arithmetic mean. Proportional changes in the geomean reflect proportional changes in the benchmarks, regardless of the relative time scales of different benchmarks. For example, given n benchmarks, if sec/op for one of them increases by a factor of 2, then the geomean of sec/op will increase by a factor of ⁿ√2.
I'll add some text to the comment to explain this.
(It also has the nice property that the delta of the geomeans is the geomean of the deltas.)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: David Chase, Dan Kortschak, Russ Cox.
Austin Clements uploaded patch set #4 to this change.
69 files changed, 3,131 insertions(+), 3,156 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Dan Kortschak (Gerrit)
Attention is currently required from: Austin Clements, David Chase, Russ Cox.
1 comment:
File cmd/benchstat/main.go:
// Finally, the last row of the table shows the geometric mean of each
// column, giving an overall picture of how the benchmarks moved.
This is why we use a geometric mean instead of an arithmetic mean. […]
Yes, I think some explanatory text about the interpretation of this and what its limitations are would be very helpful. The two most common difficult situations I would say are when the times vary greatly between the benchmarks in a set (we often see this in Gonum benchmarks when there are problem size range-specific optimisations that we want to cover) and when changes have complex interactions with performance, improving some benchmarks and making others worse while keeping overall time the same (I would imagine this is fairly common in toolchain-related benchmarks and benchmarks for the standard library).
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
David Chase (Gerrit)
Attention is currently required from: Austin Clements, Russ Cox.
7 comments:
File cmd/benchstat/internal/benchtab/builder.go:
Patch Set #3, Line 129:
tableshould this be capital-T Table?
especially, because there is a lower-t table type defined on line 37.Patch Set #3, Line 161:
ok := opts.Units.Get(unit, "assume"); okif it's not "ok", dist is "", which is not "exact", right?
Count the number of baseline benchmarks so we can
// test if later columns don't match.Is it possible that later columns could have same number of benchmarks, but different benchmarks?
Patch Set #3, Line 306:
badRatio = trueIs it worth keeping track of which cell this is, for better error reporting?
File cmd/benchstat/internal/benchtab/table.go:
Patch Set #3, Line 81:
summarizes a column of a Table.Is it worth repeating here that this appears as a cell in the final row of a table, in its corresponding column? I had a little glitch when I read "summary cell" in the Warnings document, the warnings are "for" the column, so tying this cell to its column a little more explicitly would help.
Patch Set #3, Line 103:
RowValueswould RowSummaries be a better name for this?
Patch Set #3, Line 108:
out = append(out, cell.Summary.Center)If a column is missing, would it be better to append something like a NaN here? Otherwise the summary values don't align with the columns -- it's not "every sample", exactly. Or is that not an intended case?
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Rodolfo Carvalho (Gerrit)
Attention is currently required from: Austin Clements, Russ Cox.
3 comments:
Patchset:
Looking forward to seeing this land. Nice work!
File cmd/benchstat/main.go:
Patch Set #4, Line 214:
// We can also control exactly that's used for .label from the commands/that's/what's/
Patch Set #4, Line 215:
// line. For example, support we do want to compare two files, buts/support/suppose/
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
David Chase (Gerrit)
Attention is currently required from: Austin Clements, Russ Cox.
1 comment:
File cmd/benchstat/main.go:
// give them different (perhaps shorter) labels than the full file
// names:
//
// $ benchstat A=old.txt B=new.txtThis is confusing. It implies that you did something to convert A.txt and B.txt into A and B, but what did you do? Maybe it's automatic in the obvious way, but the documentation doesn't say that.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Austin Clements, Russ Cox.
Austin Clements uploaded patch set #5 to this change.
D cmd/benchstat/testdata/namesort.golden
A cmd/benchstat/internal/benchtab/table.go
D cmd/benchstat/testdata/allnosplit.golden
A cmd/benchstat/testdata/smallSample.txt
A cmd/benchstat/testdata/units.txt
D cmd/benchstat/testdata/new4.golden
D cmd/benchstat/testdata/zero-old.txt
A cmd/benchstat/testdata/issue19634.txt
A cmd/benchstat/doc_test.go
A cmd/benchstat/testdata/units.stdout
D cmd/benchstat/testdata/all.golden
D cmd/benchstat/testdata/units.golden
D cmd/benchstat/testdata/exampleold.txt
A cmd/benchstat/testdata/crcSizeVsPoly.stdout
A cmd/benchstat/testdata/smallSample.stdout
A cmd/benchstat/internal/texttab/table.go
A cmd/benchstat/testdata/issue19565.stdout
M cmd/benchstat/testdata/new.txt
D cmd/benchstat/testdata/allnorangecsv.golden
A cmd/benchstat/internal/benchtab/builder.go
A cmd/benchstat/testdata/crc-new.txt
A cmd/benchstat/testdata/issue19565.txt
D cmd/benchstat/testdata/packagesnew.txt
D cmd/benchstat/testdata/oldnewttest.golden
A cmd/benchstat/testdata/zero.stdout
D cmd/benchstat/testdata/oldnew4html.golden
D cmd/benchstat/testdata/zero.golden
A cmd/benchstat/testdata/.gitignore
A cmd/benchstat/testdata/csvErrors.stderr
A cmd/benchstat/testdata/csvOldNew.stdout
D cmd/benchstat/testdata/oldnewgeo.golden
D cmd/benchstat/testdata/examplenew.txt
D cmd/benchstat/testdata/oldcsv.golden
D cmd/benchstat/testdata/exampleoldhtml.golden
D cmd/benchstat/testdata/units-old.txt
D cmd/benchstat/testdata/deltasort.golden
D cmd/benchstat/testdata/packages.golden
A cmd/benchstat/internal/texttab/table_test.go
A cmd/benchstat/testdata/csvErrors.stdout
M cmd/benchstat/main_test.go
M cmd/benchstat/testdata/old.txt
D cmd/benchstat/testdata/exampleold.golden
A cmd/benchstat/testdata/zero.txt
D cmd/benchstat/testdata/oldnew.golden
A benchstat/doc.go
D cmd/benchstat/testdata/allcsv.golden
D cmd/benchstat/testdata/oldnewhtml.golden
D cmd/benchstat/testdata/zero-new.txt
A cmd/benchstat/testdata/bench_test.go
D cmd/benchstat/testdata/x386.txt
A cmd/benchstat/testdata/crcIgnore.stdout
M cmd/benchstat/main.go
A cmd/benchstat/testdata/crc-old.txt
D cmd/benchstat/README.md
D cmd/benchstat/testdata/example.golden
64 files changed, 3,328 insertions(+), 3,160 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, David Chase, Russ Cox.
10 comments:
File cmd/benchstat/internal/benchtab/builder.go:
Patch Set #3, Line 129:
table
should this be capital-T Table? […]
Good point. I actually renamed lower-t table to builderTable (and cell to builderCell) to make it clear those are the parallel types used within builder.
Patch Set #3, Line 161:
ok := opts.Units.Get(unit, "assume"); okif it's not "ok", dist is "", which is not "exact", right?
Done
Count the number of baseline benchmarks so we can
// test if later columns don't match.Is it possible that later columns could have same number of benchmarks, but different benchmarks?
That is possible, yes, but it won't cause a problem here. summarizeCol compares this number against how many measurements in that column have a baseline pairing, not just the number of measurements in that column.
I've improved the comment here and also updated the issue19565 test case to cover this.
Patch Set #3, Line 306:
badRatio = trueIs it worth keeping track of which cell this is, for better error reporting?
The delta will show up as "?". Do you think this needs additional reporting?
I've enhanced TestZero to cover this case.
File cmd/benchstat/internal/benchtab/table.go:
Patch Set #3, Line 81:
summarizes a column of a Table.
Is it worth repeating here that this appears as a cell in the final row of a table, in its correspon […]
Done
Patch Set #3, Line 103:
RowValueswould RowSummaries be a better name for this?
Sure. Done.
Patch Set #3, Line 108:
out = append(out, cell.Summary.Center)
If a column is missing, would it be better to append something like a NaN here? Otherwise the summa […]
Good point. Adding NaNs will mess up the one actual use case for this. Given that this method only has one use case and that it's actually a little specialized to that use case, I've replace it with a new "RowScaler" method that collects the row values and returns the common scaler, which is the one thing we used RowValues for. I think the result is clearer.
File cmd/benchstat/main.go:
Patch Set #4, Line 214:
// We can also control exactly that's used for .label from the commands/that's/what's/
Done
Patch Set #4, Line 215:
// line. For example, support we do want to compare two files, buts/support/suppose/
Done
// give them different (perhaps shorter) labels than the full file
// names:
//
// $ benchstat A=old.txt B=new.txt
This is confusing. It implies that you did something to convert A.txt and B. […]
I believe you that you found this confusing, but I find your comment confusing, so I'm not sure what to fix. :) What are A.txt and B.txt?
The command is saying "read old.txt, but label it A in the output; and read new.txt and label it B in the output."
I rewrote this text a bit. Maybe it's clearer now?
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Michael Pratt (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Austin Clements, David Chase, Russ Cox.
Patch set 5:Code-Review +1
2 comments:
File cmd/benchstat/main.go:
Extra space here
Patch Set #5, Line 171:
"B/op"There is no B/op output, so it seems odd to include this?
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Michael Pratt, David Chase, Russ Cox.
1 comment:
File cmd/benchstat/main.go:
Patch Set #5, Line 171:
"B/op"
There is no B/op output, so it seems odd to include this?
There's only one unit in the example. :( Maybe I could turn on more measurements...
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Michael Pratt (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Austin Clements, David Chase, Russ Cox.
1 comment:
File cmd/benchstat/main.go:
Patch Set #5, Line 171:
"B/op"
There's only one unit in the example. :( Maybe I could turn on more measurements...
json/gob encoding should allocate, so perhaps just add b.ReportAllocs() to the benchmark?
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Austin Clements, David Chase, Russ Cox.
Austin Clements uploaded patch set #8 to this change.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
David Chase (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Austin Clements, Russ Cox.
3 comments:
Patchset:
Check the confidence interval value, is it 0.05 or 0.95?
File cmd/benchstat/main.go:
// give them different (perhaps shorter) labels than the full file
// names:
//
// $ benchstat A=old.txt B=new.txt
I believe you that you found this confusing, but I find your comment confusing, so I'm not sure what […]
I think this is fine, and I am now also confused by my comment.
In a comment on the benchseries POC,
https://go-review.googlesource.com/c/perf/+/380234/1..2/cmd/benchseries/main.go#b139
I think you suggested a default confidence value of 0.05, not 0.95. I'm going to guess that 0.95 is the correct default value, but just in case it is not, I'm commenting here since this is far closer to being submitted.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Austin Clements, Russ Cox.
Austin Clements uploaded patch set #9 to this change.
A benchstat/doc.go
D cmd/benchstat/README.md
A cmd/benchstat/doc_test.go
A cmd/benchstat/internal/benchtab/builder.go
A cmd/benchstat/internal/benchtab/table.go
A cmd/benchstat/internal/texttab/table.go
A cmd/benchstat/internal/texttab/table_test.go
M cmd/benchstat/main.go
M cmd/benchstat/main_test.go
A cmd/benchstat/testdata/.gitignore
D cmd/benchstat/testdata/all.golden
D cmd/benchstat/testdata/allcsv.golden
D cmd/benchstat/testdata/allnorangecsv.golden
D cmd/benchstat/testdata/allnosplit.golden
D cmd/benchstat/testdata/allnosplitcsv.golden
A cmd/benchstat/testdata/bench_test.go
A cmd/benchstat/testdata/crc-new.txt
A cmd/benchstat/testdata/crc-old.txt
A cmd/benchstat/testdata/crcIgnore.stdout
A cmd/benchstat/testdata/crcOldNew.stdout
A cmd/benchstat/testdata/crcSizeVsPoly.stdout
A cmd/benchstat/testdata/csvErrors.stderr
A cmd/benchstat/testdata/csvErrors.stdout
A cmd/benchstat/testdata/csvOldNew.stdout
D cmd/benchstat/testdata/deltasort.golden
D cmd/benchstat/testdata/example.golden
D cmd/benchstat/testdata/examplehtml.golden
D cmd/benchstat/testdata/examplenew.txt
D cmd/benchstat/testdata/exampleold.golden
D cmd/benchstat/testdata/exampleold.txt
D cmd/benchstat/testdata/exampleoldhtml.golden
A cmd/benchstat/testdata/issue19565.stdout
A cmd/benchstat/testdata/issue19565.txt
A cmd/benchstat/testdata/issue19634.stdout
A cmd/benchstat/testdata/issue19634.txt
D cmd/benchstat/testdata/namesort.golden
M cmd/benchstat/testdata/new.txt
D cmd/benchstat/testdata/new4.golden
M cmd/benchstat/testdata/old.txt
D cmd/benchstat/testdata/oldcsv.golden
D cmd/benchstat/testdata/oldnew.golden
D cmd/benchstat/testdata/oldnew4html.golden
D cmd/benchstat/testdata/oldnewgeo.golden
D cmd/benchstat/testdata/oldnewhtml.golden
D cmd/benchstat/testdata/oldnewttest.golden
D cmd/benchstat/testdata/packages.golden
D cmd/benchstat/testdata/packagesnew.txt
D cmd/benchstat/testdata/packagesold.golden
D cmd/benchstat/testdata/packagesold.txt
D cmd/benchstat/testdata/rdeltasort.golden
D cmd/benchstat/testdata/slashslash4.txt
A cmd/benchstat/testdata/smallSample.stdout
A cmd/benchstat/testdata/smallSample.txt
D cmd/benchstat/testdata/units-new.txt
D cmd/benchstat/testdata/units-old.txt
D cmd/benchstat/testdata/units.golden
A cmd/benchstat/testdata/units.stdout
A cmd/benchstat/testdata/units.txt
D cmd/benchstat/testdata/x386.txt
D cmd/benchstat/testdata/zero-new.txt
D cmd/benchstat/testdata/zero-old.txt
D cmd/benchstat/testdata/zero.golden
A cmd/benchstat/testdata/zero.stdout
A cmd/benchstat/testdata/zero.txt
64 files changed, 3,328 insertions(+), 3,160 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Russ Cox.
1 comment:
Patchset:
PS 8 -> 9 renames benchproc.Schema to Projection and Config to Key.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Russ Cox.
Austin Clements uploaded patch set #10 to this change.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Rodolfo Carvalho, Russ Cox.
Austin Clements uploaded patch set #11 to this change.
64 files changed, 3,332 insertions(+), 3,161 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Michael Pratt.
Patch set 11:Run-TryBot +1
3 comments:
File cmd/benchstat/internal/benchtab/builder.go:
Patch Set #3, Line 306:
badRatio = trueThe delta will show up as "?". Do you think this needs additional reporting? […]
Done
File cmd/benchstat/main.go:
In a comment on the benchseries POC, […]
Yes. I'm not sure if that 0.05 was a typo or if I was getting it mixed up with the alpha threshold, but this should be 0.95 for a 95% confidence interval.
File cmd/benchstat/main.go:
Patch Set #5, Line 171:
"B/op"
json/gob encoding should allocate, so perhaps just add b. […]
I tried ReportAllocs but it's mostly 0s so it's not very interesting?
I also tried adding a MB/s, but then we still only have two metrics, so filtering down to two metrics remains uninteresting, and I wanted to show the (x OR y) syntax.
I have a large rewrite of all of this documentation around a more interesting example. Maybe I should just punt to that? Of course, who knows if I'll get around to actually landing that.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Michael Pratt.
1 comment:
File cmd/benchstat/main.go:
Patch Set #5, Line 171:
"B/op"
I tried ReportAllocs but it's mostly 0s so it's not very interesting? […]
I guess I could just add a third, perhaps silly metric, like "bytes"
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Michael Pratt (Gerrit)
Attention is currently required from: Austin Clements.
Patch set 12:Code-Review +2
2 comments:
File cmd/benchstat/main.go:
Patch Set #5, Line 171:
"B/op"I tried ReportAllocs but it's mostly 0s so it's not very interesting? […]
Punt.
File cmd/benchstat/main.go:
Patch Set #12, Line 146:
// key:value - Match if key equals value.Is this from go fmt? Why does this have two tabs but everything below has one?
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
1 comment:
File cmd/benchstat/main.go:
Patch Set #12, Line 146:
// key:value - Match if key equals value.Is this from go fmt? Why does this have two tabs but everything below has one?
It must be, but I have no idea why. Fixed.
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Attention is currently required from: Austin Clements.
Austin Clements uploaded patch set #13 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Austin Clements, TryBot-Result+1 by Gopher Robot
The change is no longer submittable: TryBots-Pass is unsatisfied now.
64 files changed, 3,328 insertions(+), 3,170 deletions(-)
To view, visit change 309969. To unsubscribe, or for help writing mail filters, visit settings.