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

Jump to bottom

x/vgo: slow module lookup for single big repo #25896

Closed
utrack opened this issue Jun 14, 2018 · 6 comments
Closed

x/vgo: slow module lookup for single big repo #25896

utrack opened this issue Jun 14, 2018 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
vgo

Comments

@utrack
Copy link

utrack commented Jun 14, 2018 •
edited

Here's an example output of vgo build for a project that imports k8s api:

long log under spoiler 
...
vgo: finding k8s.io/client-go v0.0.0-20180327024835-23781f4d6632
vgo: lookup cloud.google.com/go/compute/metadata: module root is "cloud.google.com/go"
vgo: lookup cloud.google.com/go/internal: module root is "cloud.google.com/go"
vgo: lookup k8s.io/api/admissionregistration/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/admissionregistration/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/apps/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/apps/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/apps/v1beta2: module root is "k8s.io/api"
vgo: lookup k8s.io/api/authentication/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/authentication/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/authorization/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/authorization/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/autoscaling/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/autoscaling/v2beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/batch/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/batch/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/batch/v2alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/certificates/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/core/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/events/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/extensions/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/imagepolicy/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/networking/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/policy/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/rbac/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/rbac/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/rbac/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/scheduling/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/settings/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/storage/v1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/storage/v1alpha1: module root is "k8s.io/api"
vgo: lookup k8s.io/api/storage/v1beta1: module root is "k8s.io/api"
vgo: lookup k8s.io/apimachinery/pkg/api/equality: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/errors: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/meta: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/resource: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/testing: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/testing/fuzzer: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/api/testing/roundtrip: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/apimachinery: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/apimachinery/announced: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/apimachinery/registered: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/apis/meta/fuzzer: module root is "k8s.io/apimachinery"
vgo: lookup k8s.io/apimachinery/pkg/apis/meta/internalversion: module root is "k8s.io/apimachinery"
...

It seems to be very slow - each lookup for the same repo takes ~8s - looks like vgo clones the repo each time it encounters a subdirectory.

Maybe we should check if i.e. k8s.io/api was looked up already and stop any additional clones for the subdirs if we've already found the repo containing the subpackage?
@gopherbot gopherbot added this to the vgo milestone Jun 14, 2018
@utrack utrack changed the title x/vgo: slow module lookup for big repos x/vgo: slow module lookup for single big repo Jun 14, 2018
@oiooj oiooj added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 14, 2018
@kardianos
Copy link
Contributor

@utrack
I suspect this is a side effect of supporting vanilla git repos.

As more users tag repos, add go.mod files, and distributed proxies get built, this speed will drastically improve.

I suspect it is the effect of the code represented by this comment (gitrepo/fetch.go):

	// It's a prefix, and we don't have a way to make the server resolve the prefix for us,
	// or it's a full hash but also an unadvertised object.
	// Download progressively more of the repo to look for it.

@bcmills
Copy link
Contributor

bcmills commented Jun 15, 2018 •
edited

This may be fixed by https://golang.org/cl/119055. Could you try a vgo built from vgo commit 0a6cdd775a6812169f1b0e5c6bfb27d35d50cbef or later and see if it's still a problem?

@agnivade agnivade added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 16, 2018
@utrack
Copy link
Author

utrack commented Jun 18, 2018

0a6cdd is faster indeed, though as I see, same-repo lookups are still happening.

The problem is, if we fetch k8s.io/api/apps/v1beta1 and its module root is k8s.io/api we shouldn't fetch k8s.io/api/* again for other submodules. I don't think it will break any workflows/repo topologies, but I may be wrong about it (?).

@myitcv
Copy link
Member

myitcv commented Jun 18, 2018

@utrack

...we shouldn't fetch k8s.io/api/* again for other submodules.

This could well be related to #24687 (comment).

@rsc has said he's going to look again at this and related issues in the coming weeks.

@agnivade agnivade removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 18, 2018
@myitcv
Copy link
Member

myitcv commented Jul 1, 2018

@utrack can you try this again? A couple of relevant CLs have been merged in the meantime

@utrack
Copy link
Author

utrack commented Jul 2, 2018

Looks much faster now. Thank you!

@utrack utrack closed this as completed Jul 2, 2018
@golang golang locked and limited conversation to collaborators Jul 2, 2019
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.
Projects
None yet
Milestone
vgo
Development

No branches or pull requests
7 participants
@kardianos @agnivade @oiooj @myitcv @utrack @bcmills @gopherbot