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

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

cmd/vet: add compilebench to track 'go vet std' #28819

alandonovan opened this issue Nov 15, 2018 · 6 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Milestone

Comments

@alandonovan
Copy link
Contributor

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

Why does vet analysis speed belong in compilebench?

@alandonovan
Copy link
Contributor Author

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

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 bcmills added ToolSpeed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 20, 2018
@bcmills
Copy link
Contributor

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

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

@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
@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) 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