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
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Milestone

Comments

@hyangah
Copy link
Contributor

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 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed labels Sep 25, 2019
@bcmills bcmills added this to the Go1.14 milestone Sep 25, 2019
@gopherbot
Copy link

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 modified the milestones: Go1.14, Go1.13.2 Oct 3, 2019
@bcmills
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

bcmills commented Oct 4, 2019

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

@gopherbot
Copy link

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
Copy link

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>
@golang golang locked and limited conversation to collaborators Oct 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Projects
None yet
Development

No branches or pull requests

4 participants