-
-
Notifications
You must be signed in to change notification settings - Fork 378
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
Package loading time in Go 1.10.x #377
Comments
A couple of things.
|
|
I guess it is the |
Maybe it is related to this: https://github.com/golang/tools/blob/master/go/packages/golist_fallback.go#L24 It claims to be for "versions of Go earlier than 1.10.4", however it obviously gets called for 1.10.4 - I will try cleaning |
Upgrading to Go 1.10.7, recompiling and removing |
Related/caused by golang/go#29427 ? |
Without cgo, runtime is about a minute. Maybe adding a note to resource-usage about disabling cgo would be a good idea. |
Created an issue for the cgo crash: golang/go#29505 - not sure if that'll be fixed - I'll have to see if I can get our org to upgrade to Go 1.11 ;) |
Disabling tests Nothing much you can do, it seems to be mainly a problem in |
I'm not totally convinced that this is https://golang.org/issue/29427. Please run staticcheck with How long does it take to build your package (and its dependencies) with a clean cache? Does it contain massive amounts of autogenerated code? The profile above shows 10 minutes of CPU time dealing with the result of |
I don't have the code right now, but running it and excluding the tests takes ~20 seconds. Should I just run it normally with cgo and only the env variable set?
Around 10-15 seconds.
No. |
Can you get a new profile with tip x/tools? The profile you posted makes no sense; it has functions making calls that they've never made, like loadPackage calling runNamedQueries. |
I just found out why the analysis numbers are so weird. It is because the VM is swapping when doing the analysis and including tests. It crashed twice and I'm now running it without anything else running on the VM. If this crashes again, I'll increase the memory for the VM: This is the memory use once analysis starts: Go 1.11 does not use nearly as much memory. Edit: Yeah, crashed again, giving the VM 11GB memory and re-running. |
The comment is clearly wrong, and I've sent a change to fix it. We thought we'd be able to backport the go list behavior we needed to 1.10.4 from 1.11 but it was infeasible. FYI, as noted in that comment, go/packages will stop supporting Go 1.10 once Go 1.12 is released. |
Have we come to any conclusion on this issue? Are there any fixes in go/packages or a Go 1.10.x that improved the performance? |
I haven't made any changes in the fallback that could have improved it. Let me do some research and report back later today if we can get a quick win. But given that we'll be deleting the 1.10 support code when 1.12 is released in (hopefully less than) ~1 month, I don't think it's worth making heavy investment here. |
That doesn't stop people from using an old revision of the code, in case they're stuck with Go 1.10. There is a shocking amount of people stuck on old versions of Go. Of course "upgrade already!" is a very valid argument, too. |
That's true, it's hard to find the right balance... |
"Upgrade if you want better performance" is a good motivator to upgrade. |
@dmitshur except people upgraded staticcheck and got worse staticcheck performance because we started using go/packages ;) |
Yeah, it is of little use. With tests enabled it oom after loading for about 2 hours. I suspect the weird times in the initial run was because of swapping. I have been unable to complete a run since then. It may be easy for you to upgrade, but in a production environment an upgrade is always that simple. |
Summary: if there are any ideas for what we can do to improve 1.10.x performance, I'd like to hear them. Otherwise, I think of the bug in go/packages as being "unfortunate". Unfortunately, based on the information we have, it's hard to tell where the slowness is. The fallback is going to be slower than the 1.11 logic, and we won't be able to change that without doing (what I believe is too much work). If there are problems or bugs in the 1.10.x fallback logic where we're doing unnecessary work that can be identified, I'll certainly look into whether those problems are feasible to fix. I'm hoping that users who can't upgrade to 1.11+ will be able to use old versions of the tools, which continue to work. But if we've dug users into a hole where the old and new tool versions don't work, we'd like to know about it. Edit: first paragraph |
@klauspost can you compare memory usage between 1.10 and 1.11 and provide concrete numbers? That may help determine if it's unfortunate performance or a bug. If it's "just" a 2x difference, and all the timing information can be explained by swapping and thrashing, then we're probably just well advised to mark 2019.1 as "Go 1.11 only" @matloob All the tools I know about, certainly staticcheck, have old versions that don't use go/packages and work with 1.10. So at least there's no hole. |
@dominikh Thanks for the info, good to know there isn't a hole. |
It crashed with 18gb ram. I am away from my computer, but 1.11 was around
an order of magnitude less IIRC.
…On Fri, 25 Jan 2019, 00:05 Dominik Honnef ***@***.*** wrote:
@klauspost <https://github.com/klauspost> can you compare memory usage
between 1.10 and 1.11 and provide concrete numbers? That may help determine
if it's unfortunate performance or a bug. If it's "just" a 2x difference,
and all the timing information can be explained by swapping and thrashing,
then we're probably just well advised to mark 2019.1 as "Go 1.11 only"
@matloob <https://github.com/matloob> All the tools I know about,
certainly staticcheck, have old versions that don't use go/packages and
work with 1.10. So at least there's no hole.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#377 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFZs0HqsoQEsvuq-oVcAWoEtGgiCtFURks5vGjwqgaJpZM4Zmnib>
.
|
With Go 1.12 released, the removal of 1.10 support is around the corner. I'm afraid we're not going to see any more improvements for 1.10… I'm going to close this issue, hoping that we won't ever run into an issue like this again. |
I seem to have an interesting issue where running the checked is extremely slow and CPU is pegged at 100% on one core.
It seems the times in Jobs doesn't add up to the total, and obviously the 1½ hour loading time seems a bit much. It is a big package with lots of dependencies, but this does seem a bit excessive.
I restarted with
--debug.cpuprofile="profile.cpu"
to get a profile, but obviously it will take a while before the results are available.The text was updated successfully, but these errors were encountered: