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

Proposal: cmd/go: parallel execution of //go:generate #20520

Closed
tejasmanohar opened this issue May 29, 2017 · 17 comments
Closed

Proposal: cmd/go: parallel execution of //go:generate #20520

tejasmanohar opened this issue May 29, 2017 · 17 comments

Comments

@tejasmanohar
Copy link

tejasmanohar commented May 29, 2017

This is a proposal to add a -parallel <n> flag to go generate that runs //go:generate commands concurrently.

If -paralell is provided without a number, concurrency would be unbounded. If parallel is provided with an integer , that would be the max concurrent procs. If -parallel is not supplied, //go:generate commands are run serially as they are now.

If there's mutual interest, I can submit code for review, but I wanted to get feedback first.

@mvdan
Copy link
Member

mvdan commented May 29, 2017

Out of curiosity, why do you need this? go generate is something that you don't run often, so performance isn't particularly important so to speak.

Also worth noting that you could handle parallelization in a script, and then do //go:generate ./parallel.sh.

@mvdan mvdan changed the title Suggestion: Parallel execution of //go:generate Proposal: cmd/go: parallel execution of //go:generate May 29, 2017
@mvdan mvdan added the Proposal label May 29, 2017
@mvdan mvdan added this to the Proposal milestone May 29, 2017
@tejasmanohar
Copy link
Author

Personally, I think it's nice to have things fast, kinda like go build. Sometimes, to speed things up, I go find specific //go:generate statements and just run them, but it'd be nice to be able to just run it for the whole project without waiting a long time. We also run go generate in CI to keep things up-to-date, and this slows down the builds.

Your solution would work, but it's nice to keep //go:generate comments next to the affected code (e.g. //go:generate mockery ... above type declaration for struct being mocked), and parallel.sh doesn't allow for that. Plus, this would work for anyone with Go installed rather than being platform-specific.

@robpike
Copy link
Contributor

robpike commented May 30, 2017

I understand the desire but I am not keen to complicate the simple idea that is go generate. I use go generate often, but have never felt it held me back and never wanted parallelism. If you can demonstrate a widespread need for this, I might change my mind, but at the moment the need for parallel execution seems unlikely to be a common issue.

@tejasmanohar
Copy link
Author

Fair, maybe it makes sense to keep the issue open for a while to see if others are interested?

@valyala
Copy link
Contributor

valyala commented May 30, 2017

We also run go generate in CI to keep things up-to-date, and this slows down the builds.

go generate shouldn't be run in CI if you want quick reproducible builds, i.e. when all the builds on developer machines must be identical to staging builds and to prod builds. go generate during CI may result in non-reproducible builds after the corresponding tool used in go generate is updated in the CI environment.

All the output files that go generate produces must be generated and put to the repository by the developer who modifies the corresponding source files.

@tejasmanohar
Copy link
Author

tejasmanohar commented May 30, 2017

@valyala While this may often be the case, go generate running in CI doesn't imply you go get the tools in CI. The tools could be in the same repository or vendored (see retool) in a way that the versions are consistent between developers, CI, etc.

Example use case-- I've created a tool that generates typed RPC clients given server code called glue. At Segment, we run glue via go generate in CI as a part of some RPC services' release process to ensure the API clients are kept up-to-date. Through git, CI only commits if there's a diff (some releases may not result in API changes).

@mvdan
Copy link
Member

mvdan commented Jun 2, 2017

Plus, this would work for anyone with Go installed rather than being platform-specific.

Well, if a script is not portable enough, you can always write a small Go file and do go:generate go run ....

I see your point that this slightly complicates your use-case, but it seems like there's no consensus on this being a problem that is widespread enough to warrant adding complexity to go generate.

Are you sure your go generate work would get considerably faster if it ran in parallel, though? It's worth noting that CI services usually limit the amount of resources (e.g. CPU usage and cores) available, so I would be surprised if -parallel cut the run-time considerably.

@rsc
Copy link
Contributor

rsc commented Jun 5, 2017

Sorry, but no. The design decision was made that they run in serial. It's fundamental to how people use it.

@rsc rsc closed this as completed Jun 5, 2017
@tejasmanohar
Copy link
Author

tejasmanohar commented Jun 5, 2017

Got it. I understand the concern around complexity and will write an adhoc wrapper.

It's fundamental to how people use it.

Why @rsc? I don't see the connection.

@rsc
Copy link
Contributor

rsc commented Jun 14, 2017

It's allowed to write a sequence of //go:generate lines in which each builds on a step taken by the previous one.

@tejasmanohar
Copy link
Author

tejasmanohar commented Jun 14, 2017

Ah. We could parallelize on file then because I assume there's no file ordering guarantee. That said, yeah, that sounds too complex to think about. I'll have a wrapper for my own use 👍

@myitcv
Copy link
Member

myitcv commented Jun 14, 2017

@tejasmanohar

From go generate -help:

Generate processes packages in the order given on the command line,
one at a time. If the command line lists .go files, they are treated
as a single package. Within a package, generate processes the
source files in a package in file name order, one at a time. Within
a source file, generate runs generators in the order they appear
in the file, one at a time.

The order of execution is specified for go generate.

@tejasmanohar
Copy link
Author

tejasmanohar commented Jun 14, 2017

@myitcv Got it... though I hope people don't commonly depend on that, would be unintuitive in most cases IMO :)

@myitcv
Copy link
Member

myitcv commented Jun 14, 2017

@tejasmanohar I think it would be more unintuitive for people not to depend on the specified behaviour of go generate!

@tejasmanohar
Copy link
Author

tejasmanohar commented Jun 14, 2017

@myitcv I'm not suggesting we remove the ordering guarantee. I agree that it's too late for simply adding parallelization since this is documented... but I do not think there are many good use cases for relying on the file ordering guarantee of go generate (that would not be confusing to most readers).

That said, I'm happy to be proven wrong so let me know if you have good examples. The only reasonable use cases I can think of is in some sort of migration setup where you have date prefixed filenames. Otherwise, I think it's room for confusion 😬 .

@cznic
Copy link
Contributor

cznic commented Jun 14, 2017

but I do not think there are many good use cases for relying on the file ordering guarantee of go generate

go generate == extract script lines from *.go source files in well defined order and execute the resulting script.

Rarely one expects a script with no control flow constructs to not be executed top-down.

@tejasmanohar
Copy link
Author

tejasmanohar commented Jun 14, 2017

relying on the file ordering guarantee

@cznic Agreed. I meant ordering between files (e.g. if a.go or b.go's commands are run first when you go generate ./...). (Though I do think it's intuitive that if you supply a -parallel flag, ordering is not considered.)

@golang golang locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants