pessimize pluto and cfsignal builds #3892
Conversation
| # Save the PID so we can wait for it later. | ||
| static_pid="$!" | ||
| exec 1>&3 2>&4 | ||
|
|
||
| %if %{with aws_platform} || %{with aws_k8s_family} |
There was a problem hiding this comment.
Is this one of those few exceptions where it is OK to make the spec file platform or family-aware?
There was a problem hiding this comment.
Is this one of those few exceptions where it is OK to make the spec file platform or family-aware?
This is just to keep the existing conditional compile logic in effect.
When these crates don't need to be conditionally compiled any more, we can just drop the conditions and always do the AWS SDK consumer crates in a separate pass (like we do with static builds).
| exec 3>&1 4>&2 | ||
| aws_sdk_output="$(mktemp)" | ||
| exec 1>"${aws_sdk_output}" 2>&1 | ||
| RUSTFLAGS="$(echo "%{__global_rustflags_shared}" | sed -e 's,-Ccodegen-units=1,,g')" \ |
There was a problem hiding this comment.
Just for my own understanding, what's the penalty of building all the first-party crates with more than one code unit?
There was a problem hiding this comment.
Just for my own understanding, what's the penalty of building all the first-party crates with more than one code unit?
I expect it'd be in the realm of 10-20% binary size increase and an unknown performance hit. It'd push us pretty close to the current filesystem size limit for existing variants.
| # Store the output so we can print it after waiting for the backgrounded job. | ||
| exec 3>&1 4>&2 |
There was a problem hiding this comment.
nit: a comment explaining the purpose (line 330) of fd 3 and fd 4 here would have helped me out. I experienced seconds and seconds of bemusement, tbh ("fd 3: what? Non-standard-wrong-error with bonus Hadamard matrix?"). Live and learn. This may just be me.
There was a problem hiding this comment.
nit: a comment explaining the purpose (line 330) of fd 3 and fd 4 here would have helped me out.
Added a comment before the file descriptor juggling starts to explain what's going on.
On a high-end build machine, `pluto` takes 12 minutes to compile when using one codegen unit, and around 3 minutes to compile with the default of 16 codegen units. This is because the aws-sdk-ec2 crate contains a massive amount of code. Sacrifice a little code quality and spec file clarity to bring build times for this package back under control by setting up a new target directory and another background job for `pluto`. `cfsignal` is given the same treatment, even though aws-sdk-cloudformation is not as much of a problem, because they share crate dependencies that aren't used by other crates. Refactor the handling of stdout and stderr file descriptors to ensure that the `cargo build` invocation is shown when printing the results, instead of when the job is first started. Otherwise, it overlaps with the non-static, non-AWS-SDK output in a confusing way. Signed-off-by: Ben Cressey <bcressey@amazon.com>
Issue number:
N/A
Description of changes:
On a high-end build machine,
plutotakes 12 minutes to compile when using one codegen unit, and around 3 minutes to compile with the default of 16 codegen units. This is because the aws-sdk-ec2 crate contains a massive amount of code.Sacrifice a little code quality and spec file clarity to bring build times for this package back under control by setting up a new target directory and another background job for
pluto.cfsignalis given the same treatment, even though aws-sdk-cloudformation is not as much of a problem, because they share crate dependencies that aren't used by other crates.Refactor the handling of stdout and stderr file descriptors to ensure that the
cargo buildinvocation is shown when printing the results, instead of when the job is first started. Otherwise, it overlaps with the non-static, non-AWS-SDK output in a confusing way.Testing done:
Built
vmware-dev,aws-dev,aws-k8s-1.28to confirm that the expected binaries were built.Launched the
aws-k8s-1.28node which joined the cluster afterplutogenerated the usual settings.Binary size before this change:
Binary size after this change:
Definitely worse, but acceptable if this improves build times as expected.
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.