search

balena-os / balena-engine

Moby-based Container Engine for Embedded, IoT, and Edge uses
https://www.balena.io
Apache License 2.0
694 stars 66 forks source link

Consider removing the symbol tables from builds to reduce the binary size #325

Open maggie44 opened 1 year ago

maggie44 commented 1 year ago

Really interesting digging around and learning about balena-engine.

I wonder if it would be worth removing the symbol tables (https://medium.com/a-journey-with-go/go-how-to-take-advantage-of-the-symbols-table-360dd52269e5) to reduce the binary size further? It is fairly common to remove in production when debugging tools are not required.

A quick test suggests it reduces the binary size 14%.

Ideally we wouldn't want to change the code base from Moby that could create more merge commits for us. Instead, it looks like there is an option to pass in ldflags via environment variables (DOCKER_LDFLAGS).

If also seems though that it is a broken feature. I raised a PR with Moby who seem to have fixed it in the latest: https://github.com/moby/moby/issues/44729#issuecomment-1369669772

Perhaps it is something to consider when we merge the latest Moby and resolve the flags issue, or if there are other ideas for implementation, so logging here for reference.

lmbarros commented 1 year ago

The big downside of doing this is the vastly degraded troubleshooting experience. Today we get nice stack traces with proper symbols and line numbers (e.g., in case of panics). It's not very frequent for us to need this info, but when we do it is really helpful.

So, strictly from a balenaEngine perspective, I'd be against this. Of course, looking at the bigger picture it could be something to be discussed (e.g., to free some MBs from balenaOS' root partition) -- but then we should have a discussion with a broader audience to weigh the overall pros and cons.

lmbarros commented 1 year ago

Hm, but it seems this doesn't remove the info needed for stack traces: https://words.filippo.io/shrink-your-go-binaries-with-this-one-weird-trick/ . Yup, might be something to into, indeed!

maggie44 commented 1 year ago

Hm, but it seems this doesn't remove the info needed for stack traces: https://words.filippo.io/shrink-your-go-binaries-with-this-one-weird-trick/ . Yup, might be something to into, indeed!

To be honest, 14% of something already so small, a lot less when compressed for distribution, I definitely ask myself whether it's worth it. But 14% sounds nice, every little helps, my motto has always been 'KBs count', and I think thoughts out loud are always better than to myself. It's just my way of saying if we were all to discuss it more there is still a chance I would land on the 'nah, not worth it side'.

Until we reached the point where we had that upstream fix merged I couldn't think of a clean way to implement it anyway so I had seen it as more of a holding issue for the future, but if you have ideas then making things small even smaller is always a nice thing to do.

I suspect for Moby space isn't a big consideration like it is for us, but curious to see it's not already stripped. I noticed that DWARF tables are stripped though (-w is passed), so they went half way there on space reduction. I probably would have stopped there too for Moby's purposes.

olljanat commented 1 year ago

I suspect for Moby space isn't a big consideration like it is for us, but curious to see it's not already stripped. I noticed that DWARF tables are stripped though (-w is passed), so they went half way there on space reduction. I probably would have stopped there too for Moby's purposes.

Afaiu they tried that in past and had very bad issues with it. Look https://github.com/moby/moby/blob/1ef0a1b1be4d45c4df38835affc4f326bf924a6c/project/PACKAGERS.md#stripping-binaries