jmhodges / bazel_gomock

Code to create Go mocks for bazel targets using mockgen
MIT License
59 stars 29 forks source link

gomock() does a full Go compile of the library on which it depends? #2

Closed EdSchouten closed 5 years ago

EdSchouten commented 5 years ago

I notice that if I make a small change to my code, I need to spend 15-ish seconds afterwards to regenerate the appropriate mock. Running top, I see that it runs a log of Go compile processes during that time.

Is it truly necessary to spend a lot of time compiling stuff at that point? Couldn't the already compiled results be used for that?

jmhodges commented 5 years ago

Whoa, this ticket got missed. I’m not sure why that’s happening. (Maybe this is related to your PR?)

EdSchouten commented 5 years ago

Just to clarify: this is independent of the PR. Even when doing local builds (both with and without my patch being applied), I see this behaviour.

jmhodges commented 5 years ago

hm, I'm not sure how much control bazel_gomock has over this. We might have to take this over to the rules_go repo to figure out.

jmhodges commented 5 years ago

It's plausible that you're seeing the cost of reconstructing the gopath by copying files around.

Would you mind putting up a reproduction somewhere?

EdSchouten commented 5 years ago

Sure! Just check out https://github.com/EdSchouten/bazel-buildbarn and run bazel build //pkg/mock/... to be able to reproduce this.

jmhodges commented 5 years ago

Thank you!

jmhodges commented 5 years ago

Ah! I shoved this into the bazel's tracing stuff and it was really in the compile step and then I remembered why: https://github.com/golang/mock/blob/3b08ada96ed40e57fe7a70f2e4d5858e8dfd9af2/mockgen/reflect.go#L115

Mockgen works by generating and then compiling a Go program that includes your lib to generate the interfaces it needs. But, because the go binary that it runs doesn't know anything about bazel, it also has to compile all of the transitive dependencies in the bazel-made GOPATH full of source files.

Does that sound like what you were seeing?

A few possible fixes:

1) rewrite mockgen in some way to know about bazel and bazel's caches. Non-trivial. 2) have the skylark code move the .as for those libs into the right spots in the pkg dir in the GOPATH. Tricky! 3) override the go command it uses such that it copies. Maybe a cute hack? Only needs to work in the mockgen's reflect mode because the source mode doesn't use the go binary. Maybe too cute.

jmhodges commented 5 years ago

Ah! mockgen provides a -prog_only flag that generates the program and an -exec_only flag that runs the generated program and parses its input.

That means we can rig this up in bazel easily!

So, we have a 4th option:

  1. make some new auto-generated targets to use mockgen's -prog_only and -exec_only flags.
jmhodges commented 5 years ago

mockgen's flags, of course, completely rule and I think I'll email the devs to thank them for those.

jmhodges commented 5 years ago

I've posted PR #6. I've tested with your codebase and my private one and it works much, much faster.

I'm going to timebox adding some tests. The bazel testing story seems a bit rough, but I'm hoping I can at least get the golden paths.

jmhodges commented 5 years ago

That //pkg/mocks:ac, for instance, generates about 10 seconds faster for me.

Below is a trace of that target with this new mock generation code.

It was generated with bazel build --experimental_generate_json_trace_profile //pkg/mock:ac and can be viewed with chrome://tracing in a Chrome browser).

(It's really JSON, but the .txt file ending was required to get GitHub to allow it to upload.)

fixed.profile.txt

EdSchouten commented 5 years ago

Just tested it. Indeed: it speeds up builds significantly for me. Thanks a lot!