Split release profile into release and release-opt
#1495
Conversation
In `Cargo.toml`: - Add thin LTO to `release` profile. - Create `release-opt` profile with fat LTO and other slow optimizations, as well as stripping all symbols. - Remove some old commented out configuration that has been superseded by more granular configuration (separate from the above changes) In the `release.yml` CI workflow: - Build `release-opt` rather than `release` workflow. - Use an environment variable to name the `release-opt` profile so it is easy to change and identify (and make the style in which long options are passed more consistent). - Remove explicit stripping of debug symbols, since `release-opt` does that. - *Temporarily* disable the step that takes the release from draft to published, to avoid publishing more "DO-NOT-USE" releases than necessary when testing the workflow (since, for example, it is possible for them to appear in users' "Home" feeds on GitHub). This change must be undone, so the workflow will really publish.
Additionally, make current release profile potentially faster by allowing the standard optimizations, probably opt-level 2. In GitHub release mode, optimize even further as time doesn't matter quite as much there. The `build-override` with `opt-leve = 0` was removed in fear this might actually detremental for folks using `cargo install`.
There was a problem hiding this comment.
Thanks a lot for proposing the change and for acquiring numbers!
I think going to LTO="thin" should be fine and maybe it's good for a speedboost at some point, even though maybe one day we might go back to the previous setting if it turns out not to be the case.
Out of caution fear I also removed the opt-level=0 statement to rather leave it at the default.
Finally, I renamed the profile to clearly state what it is for, -github, which I think should make it less likely to be used by others - -opt sounded general enough for usage by others who want an even more optimized build.
Please do double-check my changes in case they have a chance to break something, after all I didn't really test any of this.
With the current settings, on my M1 Pro a release build can be done in 58s, which seems fine. A release-github build takes 2m30s, for comparison.
As a side-note: The inherits directive is required even though I tried not to use it, so instead I repeated all values from the release profile to assure they are not actually inherited, but rather, controlled explicitly.
There was a problem hiding this comment.
Out of
cautionfear I also removed the opt-level=0 statement to rather leave it at the default.
I've requested clarification about this in my first review comment below. The presence of build-override with a lower opt-level can affect build times, usually decreasing them unless it causes the same crates to be built more than once and otherwise sometimes increasing them. It should not affect the runtime performance of the gix and ein binaries, regardless of whether and how it is set.
Finally, I renamed the profile to clearly state what it is for
Good idea!
Please do double-check my changes in case they have a chance to break something, after all I didn't really test any of this.
I haven't rerun the workflow yet, but on inspection it doesn't seem like anything would be broken.
As a side-note: The
inheritsdirective [...}
This relates to my second review comment below. I'm not sure the current amount of repetition is clearer, since it is only repeating what release overrides explicitly from the cargo defaults--quite a bit remains implicit in both profile definitions.
It doesn't seem particularly unclear though, except with respect to the intent of one change that the second comment concerns.
I have actually tested this now, and it seems fine. |
This makes the kind of changes discussed in comments in #1475 and further in #1477. This fixes #1477, though some other fix might be preferred, so this may benefit from being changed, or further changes could be made afterward. See #1477 (comment) and #1477 (comment).
In
Cargo.toml, this adds thin LTO to thereleaseprofile, and creates a separate more aggressively optimizedrelease-optprofile with fat LTO and max 1 codegen unit. The effect on build times seems in both cases modest, but you might disagree. For more on that, see this gist where I timed the builds and #1477 (comment) (which, among other related topics, discusses it). This also removes some old commented-out code, as also discussed in #1477.In
release.yml, this takes the profile to use from an environment variable (some occurrences ofreleasehad referred to that profile while others had not), does some related other refactoring, and most significantly changes which profile is used for the GitHub releases fromreleaseto the newrelease-optprofile.The results seem okay, and in particular the Linux executables, which without configuring stripping in
Cargo.tomlmight require more explicit logic as new targets are added, show as stripped. (Thefileimplementation I am using does not seem to report the absence of symbols in macOS binaries; that affected prior runs' results as well.) That output is on assets downloaded from a draft release produced in this workflow run.In case you decide to merge this without many changes but hope for an even better approach later, I've included a comment in
Cargo.tomlstating that therelease-optprofile is not guaranteed to stick around.