x/perf/cmd/benchstat: ignore lines prefixed with `panic: `

archanaravindar commented 1 month ago

Go version

go version go1.22.5 linux/amd64

Output of `go env` in your module/workspace:

-bash-5.2$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/archanaravindar/Go/Go_master/go/bin'
GOCACHE='/home/archanaravindar/.cache/go-build'
GOENV='/home/archanaravindar/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/archanaravindar/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/archanaravindar/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/archanaravindar/Downloads/perfdash/amd/archana/Go1.22/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/archanaravindar/Downloads/perfdash/amd/archana/Go1.22/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1253104010=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I was trying to generate a benchstat comparison of the basic Go compiler benchmarks with latest version of Go1.22 and Go1.23rc1 and some of the tests face time outs on either of the Go versions and some on both in a particular set of runs.

For a subset of package directories under go/src I ran for both Go1.22.5 and Go1.23rc1, and generated two output files go1.22.txt and go1.23.txt and ran benchstat go1.22.txt and go1.23.txt

go test -c go test -bench=. -test.count=n >>output_file

What did you see happen?

In cases where only some tests time out in a particular package and generate a FAIL currently benchstat reports the results of even the tests that ran to completion on both versions separately in different sections of the file rather than as a comparison. If all the tests timed out and there is no data to compare this would be the only option. However since we have some partial data to compare a partial comparison would be useful.

What did you expect to see?

it would be helpful to atleast generate a partial comparison report for that package for those tests that ran fully. For instance in the example attached document the package bytes causes a timeout for some of the tests on both versions (obfs.23.txt and obfs.22.txt) and if we look at the benchstat output file we see all tests under bytes reported separately in different sections(obfsbenchstatout.txt). If benchstat can generate a partial comparison of atleast the tests that run to completion on both versions it would be very helpful instead of trying to write addon scripts to extract the information ourselves.

gabyhelp commented 1 month ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

archanaravindar commented 1 month ago

obfs.22.txt obfs.23.txt obfsbenchstatout.txt

dmitshur commented 1 month ago

CC @golang/runtime.

aclements commented 1 month ago

I'm not sure I fully understand the problem you're describing. Can you minimize your inputs a bit to make the problem clearer?

benchstat has no idea what packages passed or failed. My best guess is that it looks like you're capturing both stdout and stderr to one file, and benchstat is seeing the "panic: ..." line in obfs.22.txt and thinking it's benchmark metadata. Does it work as expected if you pass -ignore panic to benchstat? In general it's a problem that, if the input to benchstat isn't "clean" it can get confused by lines like this. Though "panic" may be common enough that we want to put in a special case for that.

archanaravindar commented 1 month ago

Hi @aclements -ignore panic works in this case and i am able to see the partial comparisons being generated and put next to each other alongside benchmarks that ran on only one platform as opposed to the whole list being printed separately. The only documentation link i had on benchstat was this one https://pkg.go.dev/golang.org/x/perf/cmd/benchstat and this option was not evident from the page, Thanks a bunch for the option!

IndexRuneASCII/yx-xx                xy.xyn ±  x%    xy.xxn ±  x%     -x.yx% (p=x.xxx n=x)
IndexRuneASCII/yx-xx                xx.yyn ±  x%    xx.xxn ±  y%          ~ (p=x.xxy n=x)
IndexRuneASCII/xK-xx                yxx.yn ±  x%    yxx.yn ±  x%          ~ (p=x.yyy n=x)
IndexRuneASCII/xM-xx                yxx.xµ ± yy%    xyy.xµ ± yy%          ~ (p=x.xxy n=x)
IndexRuneASCII/xxM-xx               y.yxxm ± yy%    y.xyxm ±  y%          ~ (p=x.yyx n=x)
Equal/x-xx                         x.xyyxn ±  x%   x.xyyyn ±  x%          ~ (p=x.xyy n=x)
Equal/y-xx                          xy.xxn ±  x%    xx.xxn ±  y%     -y.xx% (p=x.xxx n=x)
Equal/x-xx                          xx.yxn ±  x%    xx.xyn ±  x%     -y.xx% (p=x.xxx n=x)
Equal/y-xx                          xx.yyn ±  x%    xx.yxn ±  y%     -x.xx% (p=x.xxx n=x)
Equal/yy-xx                         xx.xxn ±  x%    xx.xyn ±  y%          ~ (p=y.xxx n=x)
Equal/yx-xx                         xx.yyn ±  x%    xx.xxn ±  y%          ~ (p=x.yxx n=x)
Equal/xx-xx                         yx.xxn ±  x%    xy.xyn ±  x%     -y.xx% (p=x.xxx n=x)
Equal/yx-xx                         yy.yyn ±  y%    yy.xxn ±  x%     -y.xy% (p=x.xxx n=x)
Equal/xK-xx                         yyx.xn ±  x%    yyy.xn ±  x%     -x.yx% (p=x.xxx n=x)
Equal/xM-xx                         yxx.yµ ± yy%    xxy.xµ ± yy%          ~ (p=x.xxy n=x)
Equal/xxM-xx                        y.yyxm ± yx%    y.xyxm ± xx%          ~ (p=x.xxy n=x)
EqualBothUnaligned/xx_x-xx          yx.yxn ±  y%    yx.yxn ±  y%     +x.yy% (p=x.xxx n=x)
EqualBothUnaligned/xx_y-xx          yx.xxn ±  x%    yx.yxn ±  y%     +x.xy% (p=x.xxx n=x)
EqualBothUnaligned/xx_x-xx          yx.yxn ±  y%    yx.yxn ±  y%     +y.yx% (p=x.xyy n=x)
EqualBothUnaligned/xx_y-xx          yx.yxn ±  x%    yx.yxn ±  y%     +y.xx% (p=x.xxx n=x)
Index/yx-xx                         xx.xyn ±  x%    xx.xyn ±  y%     -y.yy% (p=x.xxx n=x)
Index/yx-xx                         xy.yyn ±  y%    xx.xxn ±  x%     +x.yx% (p=x.xxx n=x)
Index/xK-xx                         y.yxxµ ±  x%    y.yxxµ ±  x%     +x.xx% (p=x.xxx n=x)
Index/xM-xx                         y.yxym ±  x%    y.yxym ±  x%          ~ (p=x.xyy n=x)
Index/xxM-xx                        yx.yym ±  x%    yx.yym ±  x%          ~ (p=y.xxx n=x)
IndexEasy/yx-xx                     xx.xyn ±  x%    xx.xxn ±  y%     +y.xy% (p=x.xxx n=x)
IndexEasy/yx-xx                     xx.yxn ±  y%    xy.yxn ±  y%     +x.xx% (p=x.xxx n=x)
IndexEasy/xK-xx                     yxx.yn ±  x%    yyx.yn ±  y%     +x.yy% (p=x.xxx n=x)
IndexEasy/xM-xx                     xxy.xµ ± yy%    xxy.yµ ± yx%          ~ (p=x.yxx n=x)
IndexEasy/xxM-xx                    y.xyym ± yx%    x.yxxm ±   ∞ ¹        ~ (p=y.xxx n=x+y)
Count/yx-xx                         yy.xyn ±  x%
Count/yx-xx                         yy.yxn ±  x%
Count/xK-xx                         y.yyyµ ±  x%
Count/xM-xx                         y.yxxm ±  x%

aclements commented 1 month ago

I'm glad that worked for you! It does mention "Finally, the -ignore flag can list keys that benchstat should ignore when grouping results." but it's kind of buried, and if you haven't figured out that it's treating "panic" as a key, then it's not clear that's even what you want.

I'll redirect this issue to two minor cleanups:

[ ] Ignore panic: lines in benchmark data
[ ] Document -ignore more prominently

golang / go