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.

Sign up for GitHub

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

Jump to bottom

x/tools/cover: ParseProfiles is very slow on large coverage files #32211

Closed
Katharine opened this issue May 23, 2019 · 4 comments
Closed

x/tools/cover: ParseProfiles is very slow on large coverage files #32211

Katharine opened this issue May 23, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@Katharine
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.12.4 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/ktbry/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/usr/local/google/home/ktbry/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/google-golang"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build691661056=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm using a tool that uses golang.org/x/tools/cover to parse the coverage files output by bazel coverage on kubernetes/kubernetes. The resulting (perhaps somewhat pathological) output has 3 GB of output across 827 files, totalling about 36 million blocks.

What did you expect to see?

Reasonably fast parsing of the coverage files.

What did you see instead?

The tool spends 72 seconds inside cover.ParseProfile, and 61 of those seconds are spent in ParseProfile's calls to regexp.FindStringSubmatch.

Going down the rabbit hole while I was waiting, a relatively simple (but less readable) regexp-less implementation instead spent 11 seconds in cover.ParseProfile, of which 2 seconds were spent in the replacement for the regexp.
@gopherbot gopherbot added this to the Unreleased milestone May 23, 2019
@josharian
Copy link
Contributor

Want to contribute your change?

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019
@julieqiu
Copy link
Member

/cc @ianthehat

@gopherbot
Copy link

Change https://golang.org/cl/179377 mentions this issue: cover: significantly improve the speed of cover.ParseProfiles

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@golang golang locked and limited conversation to collaborators Oct 23, 2020
gopherbot pushed a commit that referenced this issue Mar 14, 2021
@zchee @odeke-em
cmd/cover: replace code using optimized golang.org/x/tools/cover
88b8a16 
After CL 179377, this change deletes all the prior cmd/cover code
and instead vendors and type aliases code using the significantly
optimized  golang.org/x/tools/cover, which sped up ParseProfiles by
manually parsing profiles instead of a regex. The speed up was:

name         old time/op   new time/op   delta
ParseLine-12 2.43µs ± 2%   0.05µs ± 8%   -97.98% (p=0.000 n=10+9)

name         old speed     new speed       delta
ParseLine-12 42.5MB/s ± 2% 2103.2MB/s ± 7% +4853.14% (p=0.000 n=10+9)

Fixes #32211.

Change-Id: Ie4e8be7502f25eb95fae7a9d8334fc97b045d53f
Reviewed-on: https://go-review.googlesource.com/c/go/+/249759
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
@golang golang unlocked this conversation Jun 7, 2021
@gopherbot
Copy link

Change https://golang.org/cl/325909 mentions this issue: doc/go1.17: resolve TODO for cmd/cover

gopherbot pushed a commit that referenced this issue Jun 8, 2021
@odeke-em
doc/go1.17: resolve TODO for cmd/cover
f753d72 
Updates #32211

Change-Id: Ie38e831fcf557534023afd552d9394fe9e055caa
Reviewed-on: https://go-review.googlesource.com/c/go/+/325909
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
@golang golang locked and limited conversation to collaborators Jun 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@josharian @Katharine @ALTree @bcmills @julieqiu @gopherbot