-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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 test to ensure upx can compress our binaries #16706
Comments
cc @crawshaw There's compression and there's removing redundancies in the data structures, and a blurred line between them. I guess I'd like more information about what your plans might be. |
I think there's a lot to consider here. The current principle behind Go's binary layout is to minimize startup time and minimize memory use. I think that's the right goal for servers, and compressing type information or program text by default would get in the way of that. (Compressing DWARF is an all-round good idea.) There are several potential improvements to binary size that can be done within those constraints. For example we still include a string representation of types in the type information, I suspect we could pack that information better with a binary representation that the runtime and reflect packages can reconstruct into a string. (There is also the typeOff project, which I'm not done with.) However there are lots of places where binary size is more important than both startup time and memory use (or at least, it is relatively more important than on servers). I think we should consider introducing a -small mode, that is, That's where I would start with this proposal. A -small flag is a reasonably well contained little project that we can build on over time (for example, by exploring text segment compression, inlining tuning, etc.) |
Perhaps we should have a Go specific strip utility? Not sure if it's
possible to remove all the unnecessary information with a postprocessor
though.
One benefit of introducing a new tool rather than add a go build mode is
that user can have more options: strip debug info, strip symbols, strip
backtrace info, etc.
|
I don't think more options is better. I'm reluctantly proposing one after not finding a way to avoid it. :) Post processing is limiting. You can't change inlining decisions later. |
Ah, so you also want to completely disable inlining for small mode. I think
disabling inline might actually make the binary larger (the caller have to
spill all registers before the call). We just need to reduce the threshold
for inlining in the small mode.
I agree that we might not need to support code compression because upx is
already doing a good job on that. We just need to get rid of unnecessary
things in the binary. But I expect different people has different ideas on
what is unnecessary (sometimes I really want to get rid of symbolic
backtrace, but other times, I don't.)
|
This is definitely true.
Even better would be to change the cost model for inlining decisions. An OKEY node, which generates 0 bytes of code, is currently treated identically to an OAPPEND node, which generates > 100 bytes of code. |
Do we have a more comprehensive list of high-cost nodes other than OAPPEND? This seems like the sort of experiment someone ought to run. |
I looked into this. I'll file a new issue with what I know about it and we can discuss there. |
On Sun, Oct 23, 2016 at 12:15 AM, Josh Bleecher Snyder <
|
It's not easy. There are at least two obstacles. (1) Optimizations mean that the statistics can vary a lot (see related commentary in #17566). Rough patterns can be observed, though. (2) There's no good way to correlate a Node and the resulting code. I have a hacked together version that uses line number, but there are frequently multiple Nodes per line, and sometimes multiple lines per node. A better approach would be to hijack line numbers for some kind of globally unique Node numbering, but I haven't attempted that yet. Doing inlining later would obviate some of these problems but create others; again, see discussion in #17566. |
We should keep Go's binaries running with upx. We can add a cmd/go test for this (if upx is available). It doesn't seem like we should take on the problem of compressing binaries ourselves. It's not the right tradeoff most of the time (any time you care more about memory than disk), including in most cloud servers, which is the main Go target. |
Adding test to verify that upx can correctly compress our binary seems fine
to me, retitled.
|
Right now upx cannot compress go1.8 or tip binaries (at least not on linux).
So I guess we can't, for now, add a test for this. Moving to go1.10 |
upx 3.94 seems to work on go binaries generated by go1.9 and the current tip (to be go1.10), at least on linux/amd64. I've sent CL 69117 to add a test for upx compression, but at the moment upx is not installed on the builders so the test would be skipped everywhere. If we are interested in testing upx compression we should add upx to at least a couple of trybots and a couple builders (and then merge the test). |
Change https://golang.org/cl/69117 mentions this issue: |
But not on Windows for me (see upx/upx#136) |
@crazy-max thanks for letting me know. We'll have to skip the test on windows until upx is confirmed to work there. |
UPX has been working quite well on windows Go programs for me for a couple years. @crazy-max WindowsSpyBlocker appears to do some unusual things that cause that issue, I packed it with upx and it gives that error if I double click to run it, but it works fine if you run it from command line directly. |
@ctd1500 Hum ok thanks for your feedback i will check! Btw if execute multiple times the program, it works "sometimes". |
@ctd1500 What are the unusual things btw ? Thanks! |
@ALTree you can add upx to our builders: https://github.com/golang/build/blob/master/env/linux-x86-std-kube/Dockerfile |
Change https://golang.org/cl/72372 mentions this issue: |
On linux/amd64, for now. Updates #16706 Change-Id: Ib8c89b6edc73fb88042c06873ff815d387491504 Reviewed-on: https://go-review.googlesource.com/69117 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updates golang/go#16706 Change-Id: I56ec0f82119371bc7a1d6b7d8d9ba0fc7faa7fad Reviewed-on: https://go-review.googlesource.com/72372 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The new linux-x86-sid Docker image is pushed to production. Future builds should start running the new upx test, now that the upx binary is available. I'm going to close this bug for now. We can reopen if, say, the test fails. |
Change https://golang.org/cl/110817 mentions this issue: |
We have a cmd/go test ensuring that upx (an executable packer/compressor) works on linux/amd64 Go binaries. The linux-386-sid builder is built from the same dockerfile as the linux-amd64-sid builder, so upx should also already be available on the former. Since upx support 386 executables, we can enable the upx test for GOARCH=386. Updates #16706 Change-Id: I94e19ff1001de83a0386754a5104a377c72fb221 Reviewed-on: https://go-review.googlesource.com/110817 Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go 1.7 already shrank the Go binaries a lot, I think we can do
even better with judiciously used binary compression.
#11799 mentions compressed debug info, and #6993 talks
about compression of static init data. This proposal wants to
go further to do compression on data and code.
With the help from compiler, we can do much better than
standard binary compressor could do (e.g. upx). For example,
we can compress the typeinfo that only used by reflect, and
decompress then when used.
Some of the targets limit self-modifying code (iOS), so obviously
the code compression must be enabled on a target by target
basis.
The text was updated successfully, but these errors were encountered: