Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[bazel] Enable dead code stripping on macOS #537

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Conversation

fhanau
Copy link
Collaborator

@fhanau fhanau commented Apr 12, 2023

This adds the -dead_strip flag to the linker options on macOS when optimization is enabled, effectively replicating bazel's approach on Linux where --ffunction-sections --gc-sections flags are enabled by default under the opt configuration.
Based on the Mach-O binary format no compiler flag changes are needed, dead code stripping is linker-only.

optimized workerd binary size: 67.9MB -> 60.2MB

Up for discussion: I wasn't planning to enable this for fastbuild mode and only modified the .bazelrc file to show that the change won't break test cases, but based on local testing it might be a tiny bit faster since the linker generates and writes binaries with much fewer symbols. For crypto-impl-asymmetric-test, linking takes 1.5s instead of 1.8s and fastbuild binary size is down from 172MB to 116MB. While these results may not be representative – I expected dead_strip builds to be slower – this may reduce CI disk usage and improve the local build experience.

@fhanau fhanau marked this pull request as draft April 12, 2023 02:03
@fhanau fhanau force-pushed the felix/dead_strip branch from e5fe18a to a8a8607 Compare April 12, 2023 12:10
@fhanau fhanau marked this pull request as ready for review April 12, 2023 19:10
@fhanau fhanau requested a review from ohodson April 12, 2023 19:11
.bazelrc Outdated
@@ -63,6 +63,7 @@ build:unix --per_file_copt='external/zlib[~/].*\.c@-std=c90' --host_per_file_cop
build:unix --@capnp-cpp//src/kj:openssl=True --@capnp-cpp//src/kj:zlib=True --@capnp-cpp//src/kj:libdl=True

build:linux --config=unix
build:macos --linkopt=-Wl,-dead_strip --host_linkopt=-Wl,-dead_strip
Copy link
Contributor

@ohodson ohodson Apr 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice saving!

Last time I checked we were close to exceeding github disk quotas on CI and we're using fastbuild there. It seems reasonable here to enable for all non-dbg builds because of this (and the timing data). On the overall time of our builds, the small increase in linking time likely doesn't change much. If we leave a comment there, we can tune it if necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled the flag for all non-debug builds – having it for both workerd and test targets turned out to be complex but the changes in BUILD.bazel should be useful as a blueprint for future build configuration changes.

@fhanau fhanau force-pushed the felix/dead_strip branch 2 times, most recently from d73264e to 15b8982 Compare April 14, 2023 11:50
@fhanau fhanau force-pushed the felix/dead_strip branch from 15b8982 to de1de0b Compare April 14, 2023 12:44
@fhanau fhanau merged commit 7ddbe2e into main Apr 14, 2023
@fhanau fhanau deleted the felix/dead_strip branch April 14, 2023 16:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants