-
Notifications
You must be signed in to change notification settings - Fork 36
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
Enable LTO in Rust release profile #100
base: main
Are you sure you want to change the base?
Conversation
What about the development workflow where you need to build the docker image and load it to kind for local testing? |
Our makefile uses docker to build the image, which should use |
Can we have a flag or ENV variable to control this, so that we can build the docker images without LTO by default? |
I'll look into it |
I have updated the makefiles and added the following section to the By default, all our Rust-based images use the make docker-build CARGO_DEBUG=1 |
Great, I think the default should be without LTO, so when you want to cut a release build it would be something like make docker-build RELEASE=1 |
Good point. Will configure this after I fix the build-test workflow |
control-planes/mgmt_api/Dockerfile
Outdated
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
ARG RELEASE=1 |
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.
Let's make the default 0
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.
Let's make the default 0
|
||
default: docker-build | ||
|
||
docker-build: | ||
docker buildx build .. -f ./Dockerfile -t $(IMAGE_PREFIX)/kubernetes-provider:$(DOCKER_TAG_VERSION) $(DOCKERX_OPTS) | ||
docker buildx build .. --build-arg RELEASE=$(RELEASE) -f ./Dockerfile -t $(IMAGE_PREFIX)/kubernetes-provider:$(DOCKER_TAG_VERSION) $(DOCKERX_OPTS) |
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.
could we just include this arg in the $(DOCKERX_OPTS)
string?
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.
Then you would not need to modify the make files at all, you could just pass it as a parameter
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.
could we just include this arg in the $(DOCKERX_OPTS) string?
Then you would not need to modify the make files at all, you could just pass it as a parameter
|
||
default: docker-build | ||
|
||
docker-build: | ||
docker buildx build .. -f ./Dockerfile -t $(IMAGE_PREFIX)/kubernetes-provider:$(DOCKER_TAG_VERSION) $(DOCKERX_OPTS) | ||
docker buildx build .. --build-arg RELEASE=$(RELEASE) -f ./Dockerfile -t $(IMAGE_PREFIX)/kubernetes-provider:$(DOCKER_TAG_VERSION) $(DOCKERX_OPTS) |
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.
could we just include this arg in the $(DOCKERX_OPTS) string?
Then you would not need to modify the make files at all, you could just pass it as a parameter
control-planes/mgmt_api/Dockerfile
Outdated
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
ARG RELEASE=1 |
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.
Let's make the default 0
By default, all our Rust-based images use the `release` profile in builds, which sets Link-time Optimization (LTO) to true. For more information on Rust profiles and the use of LTO in Rust projects, see [Profiles](https://doc.rust-lang.org/cargo/reference/profiles.html#lto). If you wish to use the default LTO setting, you can use the `CARGO_DEBUG` parameter as follows: | ||
|
||
```sh | ||
make docker-build CARGO_DEBUG=1 |
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.
Is this still accurate?
Description
Enabling Link Time Optimization (LTO) will reduce the binary size. By setting
lto
to betrue
in the release profile, LTO will only be enabled in our release build, as we use the commandcargo install
in our dockerfiles. When we do a localcargo build
, LTO will not be enabled. To enable LTO incargo build
, run with the--release
flagType of change
Fixes: #97