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: add compilebench to track 'go vet std' #28819

Open
alandonovan opened this Issue Nov 15, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@alandonovan
Contributor

alandonovan commented Nov 15, 2018

We should update compilebench to measure the cost of cmd/vet analysis.

  go install cmd/vet
  for i in 1 2 3 4 5
  do
    go clean -cache
    time go vet std >/dev/null 2>&1 # clean build
    time go vet std >/dev/null 2>&1 # noop incremental build
  done

@alandonovan alandonovan self-assigned this Nov 15, 2018

@josharian

This comment has been minimized.

Contributor

josharian commented Nov 17, 2018

Why does vet analysis speed belong in compilebench?

@alandonovan

This comment has been minimized.

Contributor

alandonovan commented Nov 19, 2018

Russ asked me to open this issue to prevent long term performance regression in 'go vet', which entails a partial build. I'm not familiar with compilebench; perhaps @rsc can clarify.

@bcmills

This comment has been minimized.

Member

bcmills commented Nov 20, 2018

vet also runs when compiling tests, so users won't necessarily distinguish between “the compiler is taking a long time to build this test” and “vet is taking a long time to analyze this package”.

@bcmills

This comment has been minimized.

Member

bcmills commented Nov 20, 2018

@alandonovan, what milestone should this be? (Do you intend to do it for 1.12, or just filing for later?)

@josharian

This comment has been minimized.

Contributor

josharian commented Nov 20, 2018

'go vet', which entails a partial build

Is this a partial build using cmd/compile? Or a typechecking via go/packages or go/types? (Sorry, I haven't tracked any of the recent changes.) If the former, I don't see why the regular compilebench benchmarks don't cover it. If the latter, I don't think compilebench is the right home for it, since it is about cmd/compile. Similarly, if the question is about vet performance once the partial build is complete, I don't think compilebench is the right home.

I definitely agree that we should track this performance. It's just a question of where and how.

vet also runs when compiling tests, so users won't necessarily distinguish between “the compiler is taking a long time to build this test” and “vet is taking a long time to analyze this package”.

Worth noting that the same compilation is required for vet and for actually compiling and linking the test, so adding in vet isn't actually much of a change here.

@alandonovan

This comment has been minimized.

Contributor

alandonovan commented Nov 20, 2018

@josharian: the new analysis tools in x/tools may run in two modes: "above" the build system, using x/tools/go/packages (implemented by multichecker), or "below" the build system, through "go vet -vettool" (implemented by unitchecker). The cmd/vet in GOROOT works only in the latter mode, to avoid depending on multichecker and thence go/packages. The command 'go vet p' compiles everything package p depends on, but not p itself. I agree there's no obvious way to separate the contributions of cmd/compile and cmd/vet to the time of go vet. It seems as hard as the usual problem of measuring the critical path contribution of some task in a parallel build.

@bcmills I don't think it's a 1.12 blocker. I don't have a good sense of where this benchmark belongs; I filed the issue at Russ's request.

@bcmills bcmills added this to the Unplanned milestone Nov 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment