Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/cmd/benchcmp: warn about outliers for -best mode. #8182

Closed
minux opened this issue Jun 11, 2014 · 5 comments
Closed

x/tools/cmd/benchcmp: warn about outliers for -best mode. #8182

minux opened this issue Jun 11, 2014 · 5 comments

Comments

@minux
Copy link
Member

minux commented Jun 11, 2014

for example, given these data (those are real data):
// new.txt
BenchmarkGoroutineForRange       500       3915445 ns/op
BenchmarkGoroutineForRange       500       3873353 ns/op
BenchmarkGoroutineForRange       500       3850778 ns/op
BenchmarkGoroutineForRange       500       3903556 ns/op
BenchmarkGoroutineForRange       500       3768451 ns/op

// old.txt
BenchmarkGoroutineForRange       500       3922159 ns/op
BenchmarkGoroutineForRange      1000       1766253 ns/op
BenchmarkGoroutineForRange       500       3916289 ns/op
BenchmarkGoroutineForRange       500       3903132 ns/op
BenchmarkGoroutineForRange       500       3890627 ns/op

benchcmp -best reports this:
BenchmarkGoroutineForRange                1766253       3768451       +113.36%

However, the 2nd run of old.txt is obviously an outliner.
benchcmp should track and warn about this kind of strange data points.

I propose this criteria: if the max. difference between different runs in the same
file is on the same order of difference than that of best of old and new runs,
benchcmp should warn.
@josharian
Copy link
Contributor

Comment 1:

Seems reasonable to warn in such cases. I'm not sure I understand the proposed
criterion, though.
I'm also surprised by the outlier itself. Was there a code change there?
We'd probably want to move the implementation of -best from parsing to comparison to
accomplish this.

@minux
Copy link
Member Author

minux commented Jun 11, 2014

Comment 2:

let me rephrase the criteria more formally:
if the std. deviation of old or new data points is too large (compared
to the value themselves), then warn.
the problem is how large is too large? 100%? 50%? 30%? 10%?
PS: the data is from the m to g->m change that I mentioned in the ARM
NaCl design doc on golang-dev.

@josharian
Copy link
Contributor

Comment 3:

Oh, so something like the coefficient of variation? Devising correct statistical tests
is way out of my depth.
As for how much is too much, I'd just naively observe a bunch of values from normal runs
and hope that the right number is obvious. And in this case, I'd say false positives are
better than false negatives.

@rsc
Copy link
Contributor

rsc commented Sep 15, 2014

Comment 4:

Labels changed: added release-none, removed release-go1.4.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed the release-none label Apr 10, 2015
@rsc rsc changed the title go.tools/cmd/benchcmp: warn about outliers for -best mode. x/tools/cmd/benchcmp: warn about outliers for -best mode. Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-tools label Apr 14, 2015
@ALTree
Copy link
Member

ALTree commented Jun 28, 2019

This is a benchcmp issues that is solved by using benchstat, which is what we recommend now anyway. Closing here.

@ALTree ALTree closed this as completed Jun 28, 2019
@golang golang locked and limited conversation to collaborators Jun 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants