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

cmd/go: `go mod download -json` is a lot slower with go1.13 #34533

Closed
hyangah opened this issue Sep 25, 2019 · 6 comments
Closed

cmd/go: `go mod download -json` is a lot slower with go1.13 #34533

hyangah opened this issue Sep 25, 2019 · 6 comments

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Sep 25, 2019

The feature requested in #32239 was added to go1.13 (per https://golang.org/cl/183841).
That makes the go mod download -json command populates the Latest boolean field of the json output by performing an extra query to check the latest version of the module.

It turned out the overhead of this extra query is significant in some cases,
especially when the module has many incompatible versions tags. E.g. 'k8s.io/kubernetes' or 'github.com/openshift/origin'.

$ GOPATH=`mktemp -d` GOPROXY=direct GONOSUM=* time  ~/go/bin/go1.13 mod download -json github.com/openshift/origin@v1.3.0-alpha.0
go: finding github.com/openshift/origin v1.3.0-alpha.0
go: finding github.com/openshift/origin v4.1.0+incompatible
...
154.54user 9.60system 1:42.54elapsed 160%CPU (0avgtext+0avgdata 482292maxresident)k
112inputs+2314104outputs (0major+229264minor)pagefaults 0swaps

$ GOPATH=`mktemp -d` GOPROXY=direct GONOSUM=* time  ~/go/bin/go1.13beta1 mod download -json github.com/openshift/origin@v1.3.0-alpha.0
go: finding github.com/openshift/origin v1.3.0-alpha.0
...
12.01user 1.62system 0:15.11elapsed 90%CPU (0avgtext+0avgdata 130020maxresident)k
360inputs+414112outputs (0major+18928minor)pagefaults 0swaps

This is a huge regression compared to the benefit from the new functionality.
Let's consider to provide an option to disable, or disable it by default if optimizing the 'latest' query is non trivial.

@bcmills @jayconrod @katiehockman @heschik

@bcmills bcmills added this to the Go1.14 milestone Sep 25, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 3, 2019

Change https://golang.org/cl/198699 mentions this issue: Revert "cmd/go: add a Latest field to the output of 'go mod download -json'"

@katiehockman katiehockman added the modules label Oct 3, 2019
@katiehockman katiehockman modified the milestones: Go1.14, Go1.13.2 Oct 3, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Oct 3, 2019

@gopherbot, please backport to Go 1.13.2: this feature caused a significant performance regression for large repositories, and was newly introduced and fairly obscure (so is unlikely to have broad usage at this point).

The only likely users are other module proxy implementations; we're asking them explicitly if they care about this field in a golang-module-proxies thread.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 3, 2019

Backport issue(s) opened: #34679 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Oct 4, 2019

No objections here or on the email thread, so we're going to go ahead and revert.

@gopherbot gopherbot closed this in 961837d Oct 4, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 4, 2019

Change https://golang.org/cl/199079 mentions this issue: [release-branch.go1.13] Revert "cmd/go: add a Latest field to the output of 'go mod download -json'"

gopherbot pushed a commit that referenced this issue Oct 4, 2019
…put of 'go mod download -json'"

This reverts CL 183841.

Updates #34533
Fixes #34679

Reason for revert: Introduced a significant performance regression for repos with many incompatible-version tags.

Change-Id: I75d7fd76e6e1a0902b114b00167b38439e0f8221
Reviewed-on: https://go-review.googlesource.com/c/go/+/198699
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 961837d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/199079
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 31, 2019

Change https://golang.org/cl/204439 mentions this issue: cmd/go/internal/modfetch: prune +incompatible versions more aggressively

gopherbot pushed a commit that referenced this issue Nov 5, 2019
codeRepo.Versions previously checked every possible +incompatible
version for a 'go.mod' file. That is wasteful and counterproductive.

It is wasteful because typically, a project will adopt modules at some
major version, after which they will (be required to) use semantic
import paths for future major versions.

It is counterproductive because it causes an accidental
'+incompatible' tag to exist, and no compatible tag can have higher
semantic precedence.

This change prunes out some of the +incompatible versions in
codeRepo.Versions, eliminating the “wasteful” part but not all of the
“counterproductive” part: the extraneous versions can still be fetched
explicitly, and proxies may include them in the @v/list endpoint.

Updates #34165
Updates #34189
Updates #34533

Change-Id: Ifc52c725aa396f7fde2afc727d0d5950acd06946
Reviewed-on: https://go-review.googlesource.com/c/go/+/204439
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.