Add runc_nocriu build tag to opt out of c/r
#4546
Conversation
|
See also: #3959 I guess |
| @@ -1,7 +1,5 @@ | |||
| package libcontainer | |||
There was a problem hiding this comment.
Is it possible to also move the CriuPageServerInfo stuff from checkpoint.go so we can mark this file as //go:build !runc_nocr as well? I guess it doesn't matter for the size of the final binary, but it is a little odd to keep this even with runc_nocr.
Actually, would it be possible to move criuOptions to a !runc_nocr file, or would that be too difficult?
There was a problem hiding this comment.
I did that initially, but when I realized that runc checkpoint and runc restore commands should be kept intact, so when a user runs those, they see a useful error like "c/r not compiled in" rather than No help topic for 'checkpoint' (which is urfave/cli way to say no such command).
Same for all the checkpoint/restore options: I kept those so that a valid command like runc checkpoint --leave-running XXX results in a useful "c/r not compiled in" error rather than "flag provided but not defined: -leave-running`.
In order to keep both commands and their options, we have to keep the data structures, too. We're lucky there aren't that many.
Sure, we can duplicate the whole thing with commands and options but without data structures, but that would be a mere duplication.
So, for the sake of both simplicity and user experience I keep all the CLI parsing (and the associated data structures) in place. Hope it makes sense.
kolyshkin
changed the title
runc_nocr build tag to opt out of c/rrunc_nocriu build tag to opt out of c/r
Probably a good idea yes, to at least make sure we have the right build-tags in all files |
Commit 7f64fb4 made the main package, and runc/libcontainer's CriuOpts depend on criu/rpc. This is not good; among the other things, it makes it complicated to make c/r optional. Let's switch CriuOpts.ManageCgroupsMode to a string (yes, it's an APIt breaking change) and move the cgroup mode string parsing to libcontainer. While at it, let's better document ManageCgroupsMode. Signed-off-by: Kir Kolyshkin <[email protected]>
This allows to make a 17% smaller runc binary by not compiling in checkpoint/restore support. It turns out that google.golang.org/protobuf package, used by go-criu, is quite big, and go linker can't drop unused stuff if reflection is used anywhere in the code. Currently there's no alternative to using protobuf in go-criu, and since not all users use c/r, let's provide them an option for a smaller binary. For the reference, here's top10 biggest vendored packages, as reported by gsa[1]: $ gsa runc | grep vendor | head │ 8.59% │ google.golang.org/protobuf │ 1.3 MB │ vendor │ │ 5.76% │ github.com/opencontainers/runc │ 865 kB │ vendor │ │ 4.05% │ github.com/cilium/ebpf │ 608 kB │ vendor │ │ 2.86% │ github.com/godbus/dbus/v5 │ 429 kB │ vendor │ │ 1.25% │ github.com/urfave/cli │ 188 kB │ vendor │ │ 0.90% │ github.com/vishvananda/netlink │ 135 kB │ vendor │ │ 0.59% │ github.com/sirupsen/logrus │ 89 kB │ vendor │ │ 0.56% │ github.com/checkpoint-restore/go-criu/v6 │ 84 kB │ vendor │ │ 0.51% │ golang.org/x/sys │ 76 kB │ vendor │ │ 0.47% │ github.com/seccomp/libseccomp-golang │ 71 kB │ vendor │ And here is a total binary size saving when `runc_nocriu` is used. For non-stripped binaries: $ gsa runc-cr runc-nocr | tail -3 │ -17.04% │ runc-cr │ 15 MB │ 12 MB │ -2.6 MB │ │ │ runc-nocr │ │ │ │ └─────────┴──────────────────────────────────────────┴──────────┴──────────┴─────────┘ And for stripped binaries: │ -17.01% │ runc-cr-stripped │ 11 MB │ 8.8 MB │ -1.8 MB │ │ │ runc-nocr-stripped │ │ │ │ └─────────┴──────────────────────────────────────────┴──────────┴──────────┴─────────┘ [1]: https://github.com/Zxilly/go-size-analyzer Signed-off-by: Kir Kolyshkin <[email protected]>
This PR already includes building runc with |
Yes, but it's only a build test, but not an integration test.
For this PR, I think we would not introduce any regressions, so it's not a big necessary to write an integration test. |
🙈 that's my bad for commenting from my phone; I didn't recall if we added that, and thought we didn't based on the comment before that.
Yeah, I don't think we need that, as (as you mentioned) in this case the test would be purely skipping things that are not used? So from my perspective, this one should be good to go |
This allows to make a 17% smaller runc binary by not compiling in
checkpoint/restore support.
It turns out that google.golang.org/protobuf package, used by go-criu,
is quite big, and go linker can't drop unused stuff if reflection is
used anywhere in the code.
Currently there's no alternative to using protobuf in go-criu, and since
not all users use c/r, let's provide them an option for a smaller
binary.
For the reference, here's top10 biggest vendored packages, as reported
by gsa1:
And here is a total binary size saving when
runc_nocriuis used.For non-stripped binaries:
And for stripped binaries: