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/vet: switch to new srcimporter #19332

Closed
josharian opened this issue Feb 28, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@josharian
Copy link
Contributor

commented Feb 28, 2017

Go 1.9 has a new source-based importer. I'd like to switch cmd/vet to it. This will resolve a host of open issues.

It should also make cmd/vet/all a lot faster, which would be good, since @bradfitz is a bit concerned about kubernetes CPU capacity at the moment.

cc @robpike @griesemer @valyala

@josharian josharian added the Builders label Feb 28, 2017

@josharian josharian added this to the Go1.9 milestone Feb 28, 2017

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2017

I'd like to complete this work first; i.e., also implement an "gc/auto" importer which automatically decides whether to use installed packages or source (whichever is newer). And then make this "auto" importer the default importer. As a consequence, programs that just use the default importer wouldn't have to change at all.

Any objections?

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented Mar 1, 2017

Sounds great.

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2017

Per #11415 (comment) I think this should go forward with the "source" importer, rather than waiting for an "auto" importer.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 2, 2017

CL https://golang.org/cl/37690 mentions this issue.

@myitcv

This comment has been minimized.

Copy link
Member

commented Mar 2, 2017

@josharian

It should also make cmd/vet/all a lot faster

I might be missing something here, but won't it do the opposite? i.e. make cmd/vet run slower?

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2017

See the numbers in https://go-review.googlesource.com/c/37691/.

cmd/vet/all currently has to do 'go install' in order to typecheck the files, which invokes cmd/compile, which is (unsurprisingly) heavier-weight than go/types. For cases in which the relevant packages have already been compiled, using the installed object files will be faster. That will remain the default for cmd/vet.

@myitcv

This comment has been minimized.

Copy link
Member

commented Mar 2, 2017

Thanks for the link: I understand the context now.

For cases in which the relevant packages have already been compiled, using the installed object files will be faster.

That's what I was getting at. My concern here was that for situations where packages are up to date there would be a needless hit by vet being moved over to the source importer (the exact use case of the "auto" importer)

... That will remain the default for cmd/vet.

Thanks for clarifying.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.