-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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/go: add timing information to -debug-actiongraph #15736
Comments
I think this is pretty cool. On Wed, May 18, 2016 at 3:04 PM, Josh Bleecher Snyder <
|
Good point. But parallelizing compilation within a package requires removing a bunch of remaining global variables in the compiler, which might take longer than Josh's proposal? |
On Wed, May 18, 2016 at 3:33 PM, Brad Fitzpatrick [email protected]
|
Indeed. However, (a) there will always be inefficiencies in getting work scheduled, so doing both could help nevertheless and (b) as @bradfitz observed, scheduling compilation processes better helps a lot more if you start to ship compilation off over the network to other machines.
cmd/go already parallelizes compiles, just not super efficiently; it already handles errors and juggles the spiderweb. That said, I rather dislike the cmd/go codebase, in part for this reason. :) cmd/go chooses ncpus as the number of parallel compiles to run, which can be altered with Also worth noting is that not all of cmd/go's work is in the compiler--cgo and resulting c compilations can be a big part of compile times, and those are not likely to be as core-saturating as cmd/compile anytime soon. So I'm inclined to say it is still worth pursuing both paths. And we have the tools to control concurrency on both fronts ( |
On Wed, May 18, 2016 at 3:51 PM, Josh Bleecher Snyder <
I'm not saying we shouldn't do this. I'd really like to have the tracing
|
I fear this line of investigation is mostly my fault because of my focus on Without excluding the excellent investigation that Josh has done, I think Thanks Dave On Thu, 19 May 2016, 09:11 Keith Randall, [email protected] wrote:
|
Thanks, everyone, for the input, very helpful. I've now also heard privately that this tracing was helpful, for a non-Go-contributor. So based on this conversation, I'll plan to move forward on adding tracing to cmd/go early in 1.8, and put the other proposal (#15734) on hold until cmd/compile is more concurrent, at which point we can re-evaluate. Is there an issue or place where I can track progress on (and contribute to) making cmd/compile more concurrent? |
I don't know of a issue for that. I'll open one. On Thu, May 19, 2016 at 10:44 AM, Josh Bleecher Snyder <
|
@josharian, this is marked early. Got a CL ready? |
A have a CL at 70%. Will put it at top of my queue. I also don't see harm in removing the early milestone; this is not as intrusive or risky as I originally thought. |
This CL just made it to the top of my queue for cleanup and mailing, but not quite fast enough. I'm out on vacation until late October. This might or might not happen for Go 1.8. If not 1.8, though, 1.9; I do intend to see it through. If anyone wants to pick up the ball and run with it while I'm away, the WIP code is at this unstable link. The major TODOs are:
|
I'm going to move this to Go1.8Maybe since you might or might not get to it for 1.8 but it's not going to block the 1.8 release. Good luck with your vacation, and I am also looking forward to seeing this feature in Go! We can move this to Go1.9 if it misses 1.8. |
There's now an (intentionally) undocumented |
CL 177138 adds timing. |
In order to get insight into how cmd/go schedules work, I added the ability to generate a trace using the same viewer as cmd/trace. There is a screenshot and sample html output at #15734. It was generated using the command
go build -a -trace std.html std
. A partial implementation (doesn't trace downloads, test running, etc.) can be seen at unstable url.This issue is to discuss whether this would be worth adding to cmd/go in 1.8.
Advantages:
Disadvantages:
Feedback requested.
The text was updated successfully, but these errors were encountered: