-
Notifications
You must be signed in to change notification settings - Fork 5.1k
build: Add option for size-optimized binary #6960
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
Conversation
Add an option to build a binary that is optimized for size with -Os. Signed-off-by: Tony Allen <tallen@lyft.com>
Signed-off-by: Tony Allen <tallen@lyft.com>
bazel/envoy_build_system.bzl
Outdated
@@ -75,6 +75,9 @@ def envoy_copts(repository, test = False): | |||
}) + select({ | |||
repository + "//bazel:disable_signal_trace": [], | |||
"//conditions:default": ["-DENVOY_HANDLE_SIGNALS"], | |||
}) + select({ | |||
repository + "//bazel:optimize_bin_size": ["-Os"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you compare this vs passing --copt=-Os
to bazel? envoy_copts
doesn't apply to dependencies but CLI copt is. So I think --copt=-Os
may produce smaller binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, we need to compile all dependencies with -Os also. Also, can we make sure we are compiling with symbols also? They can be stripped after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, awesome to see this in progress.
/wait
bazel/envoy_build_system.bzl
Outdated
@@ -75,6 +75,9 @@ def envoy_copts(repository, test = False): | |||
}) + select({ | |||
repository + "//bazel:disable_signal_trace": [], | |||
"//conditions:default": ["-DENVOY_HANDLE_SIGNALS"], | |||
}) + select({ | |||
repository + "//bazel:optimize_bin_size": ["-Os"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, we need to compile all dependencies with -Os also. Also, can we make sure we are compiling with symbols also? They can be stripped after.
Using I left the option for compiling just Envoy in case somebody wants to do that. Let me know if you think I should rip it out. |
I would just rip it out. Can you also document this new stuff in https://github.com/envoyproxy/envoy/blob/master/bazel/README.md? Thank you! /wait |
Yeah, I think you can rip it out and add a config to |
Signed-off-by: Tony Allen <tallen@lyft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks few more comments.
/wait
Signed-off-by: Tony Allen <tallen@lyft.com>
Signed-off-by: Tony Allen <tallen@lyft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, nice addition and cleanup. A few small comments.
/wait
Signed-off-by: Tony Allen <tallen@lyft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks. Few more comments.
/wait
Signed-off-by: Tony Allen <tallen@lyft.com>
Signed-off-by: Tony Allen <tallen@lyft.com>
@tonya11en in the future friendly request to not force push. It makes reviews much harder. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you!
* master: (65 commits) proto: Add PATCH method to RequestMethod enum (envoyproxy#6737) exe: drop unused deps on zlib compressor code (envoyproxy#7022) coverage: fix some misc coverage (envoyproxy#7033) Enable proto schema for router_check_tool (envoyproxy#6992) stats: rework stat sink flushing to centralize counter latching (envoyproxy#6996) [test] convert lds api test config stubs to v2 (envoyproxy#7021) router: scoped rds (2c): implement scoped rds API (envoyproxy#6932) build: Add option for size-optimized binary (envoyproxy#6960) test: adding an integration test framework for file-based LDS (envoyproxy#6933) doc: update obsolete ref to api/XDS_PROTOCOL.md (envoyproxy#7002) dispatcher: faster runOnAllThreads (envoyproxy#7011) example: add csrf sandbox (envoyproxy#6805) fix syntax of gcov exclusion zone. (envoyproxy#7023) /runtime_modify: add support for query params in body (envoyproxy#6977) stats: Create stats for http codes with the symbol table. (envoyproxy#6733) health check: fix more fallout from inline deletion change (envoyproxy#6988) Max heap fix (envoyproxy#7016) Add support to unregister from lifecycle notifications (envoyproxy#6984) build spdy_core_alt_svc_wire_format (envoyproxy#7010) ext_authz: Make sure initiateCall only called once (envoyproxy#6949) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Add an option to build a binary that is optimized for size with
-Os
. Appending--define optimize_binary_size=enabled
to Bazel build will override the optimization level.Risk Level:
Low
Testing:
Manual testing.
Docs Changes:
Release notes and build docs.
Release Notes:
Done.