proposal: cmd/vet: refactor to be cheaper

[robpike]

The structure of vet is to have a set of checks, each of which walks the whole AST. This is expensive. It might be possible to refactor it to walk the AST once and do all the work in one pass, or at least to fold many of the locally identifiable issues into a single such pass.

For example, #19675 is not worth a full AST walk but is worth a function call on every constant.

[bradfitz]

How expensive is a no-op walk?

[josharian]

I'm confused. I thought vet already did a single AST walk and called relevant checks for each node:

https://github.com/golang/go/blob/master/src/cmd/vet/main.go#L500

Of course, each checker might do non-trivial work once it is running, but I don't think that is a big problem at the moment.

[robpike]

httpresponse.go rewalks the whole file. I thought there were others, but there aren't.

There's something nagging at me here but I clearly haven't expressed it well.

[haya14busa]

maybe #8338 is related?

[rsc]

One experiment that might help (volunteers welcome): implement the -0 check in #19675 and measure how much slower vet gets. If it's significant, can it be improved?

[josharian]

One experiment that might help (volunteers welcome): implement the -0 check in #19675 and measure how much slower vet gets.

Did that. CL 38779. No detectable performance change in vet.

[rsc]

Thanks for the data. We'd forgotten that there's one shared AST walk for most checkers.

[josharian]

If desired, this could be re-opened and repurposed as an issue to make the httpresponse checker partake of that single shared walk. Although I strongly suspect that that is a low priority on the vet performance front.

[gopherbot]

Change https://golang.org/cl/38779 mentions this issue: cmd/vet: add check for -0.0