-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Tracking issue for custom-named-profile (RFC 2678) #6988
Comments
Support for named profiles (RFC 2678) Tracking issue: #6988 Implementation according to the [RFC](https://github.com/rust-lang/rfcs/blob/master/text/2678-named-custom-cargo-profiles.md).
A few things that came up during the PR: How should target-specific profiles be chosen?Stable cargo uses a variety of factors to choose the profile per compilation unit:
Dependencies are also special, in that when building a test, the test itself will use the "test" profile, but the dependencies will use "dev". This is done under the assumption that if the Some people have found this confusing, and it is a little hard to document clearly. However, it is intended to be intuitive (test targets use the "test" profile). The addition of named profiles makes this even more complex and possibly confusing. Try to answer the question of which profile each unit should use for the following:
One option is to toss away the per-target logic, and use a uniform profile for everything. Another option is to try to have some elaborate system that tries to retain something like the old behavior. Another option is to have some sort of target-specific profiles. That is, This is a difficult problem, and I'm not really sure where to go with it. Add named profile support to
|
1849: feat: no debug symbols as default and add a command to build with debug symbols r=quake a=yangby-cryptape 1. [Stable releases of cargo don't support custom profiles.](../../../rust-lang/cargo/issues/6988) 2. We can't use another manifest file in same directory since `error: the manifest-path must be a path to a Cargo.toml file`. 3. I try to put a debug version of `Cargo.toml` in another directory, it's conflicted with the original `Cargo.toml`. Each crate should be in only one workspace. 4. Add arguments into `RUSTFLAGS` to control `rustc`, but it's unable to achieve the same effect. According to [The Cargo Book](https://doc.rust-lang.org/cargo/reference/manifest.html#the-profile-sections): `debug = true` is equivalent to `-C debuginfo=2` compiler flag. According to `rustc --help`, `-g` is equivalent to `-C debuginfo=2`. But when use `debug = true`, the size of debug symbols in executable file is bigger than `-g` (or `-C debuginfo=2`), ~~and `debug = true` will enable some other debug features, for example: `debug_assertions = true` (I found it in [rust source code](https://github.com/rust-lang/rust/blob/695fe965173795f9242dfcad6d1c07d7a17b106a/src/bootstrap/config.rs#L631-L632), but it doesn't works when I test it)~~. So, I think `debug = true` is better than `-g` or `-C debuginfo=2` for debugging. - Execute `cargo build --release` without `debug = true`. The size of `ckb` is 36876256 Bytes (36MB). After strip, it will be 26102560 Bytes (25MB). - Execute `cargo build --release` with `debug = true`. The size of `ckb` is 439163736 Bytes (419MB). After strip, it will be 26098544 Bytes (25MB). - Execute `RUSTFLAGS="-g" cargo build --release` without `debug = true`. The size of `ckb` is 366969536 Bytes (350MB). After strip, it will be 26155888 Bytes (25MB). - Execute `RUSTFLAGS="-C debuginfo=2" cargo build --release` without `debug = true`. The size of `ckb` is 26151792 Bytes (351MB). After strip, it will be 26098544 Bytes (25MB). Use same command always get the same size executable file. **It's deterministic.** #### Appendix <details><summary>The stripped part for stripping an executable file which compile by <code>cargo build --release</code> without <code>debug = true</code>.</summary> <pre> Sections: Idx Name Size VMA LMA File off Algn Flags 27 .comment 0000009a 0000000000000000 0000000000000000 018e41fa 2**0 CONTENTS, READONLY 28 .debug_aranges 00000270 0000000000000000 0000000000000000 018e4294 2**0 CONTENTS, READONLY, DEBUGGING 29 .debug_pubnames 0003bcbd 0000000000000000 0000000000000000 018e4504 2**0 CONTENTS, READONLY, DEBUGGING 30 .debug_info 00150220 0000000000000000 0000000000000000 019201c1 2**0 CONTENTS, READONLY, DEBUGGING 31 .debug_abbrev 00006993 0000000000000000 0000000000000000 01a703e1 2**0 CONTENTS, READONLY, DEBUGGING 32 .debug_line 000a3beb 0000000000000000 0000000000000000 01a76d74 2**0 CONTENTS, READONLY, DEBUGGING 33 .debug_frame 00001230 0000000000000000 0000000000000000 01b1a960 2**3 CONTENTS, READONLY, DEBUGGING 34 .debug_str 000b5275 0000000000000000 0000000000000000 01b1bb90 2**0 CONTENTS, READONLY, DEBUGGING 35 .debug_loc 001579db 0000000000000000 0000000000000000 01bd0e05 2**0 CONTENTS, READONLY, DEBUGGING 36 .debug_macinfo 00000031 0000000000000000 0000000000000000 01d287e0 2**0 CONTENTS, READONLY, DEBUGGING 37 .debug_pubtypes 00019c31 0000000000000000 0000000000000000 01d28811 2**0 CONTENTS, READONLY, DEBUGGING 38 .debug_ranges 00098440 0000000000000000 0000000000000000 01d42442 2**0 CONTENTS, READONLY, DEBUGGING </pre> </details> <details><summary>Size of debug symbols in executable files for different compilation conditions.</summary> <pre> Sections: Idx Name Size VMA LMA File off Algn Flags // debug = true ... omitted ... 29 .debug_aranges 000002a0 0000000000000000 0000000000000000 018e3294 2**0 CONTENTS, READONLY, DEBUGGING 30 .debug_pubnames 0225e9f0 0000000000000000 0000000000000000 018e3534 2**0 CONTENTS, READONLY, DEBUGGING 31 .debug_info 0783474a 0000000000000000 0000000000000000 03b41f24 2**0 CONTENTS, READONLY, DEBUGGING 32 .debug_abbrev 001a9b4b 0000000000000000 0000000000000000 0b37666e 2**0 CONTENTS, READONLY, DEBUGGING 33 .debug_line 013b5e42 0000000000000000 0000000000000000 0b5201b9 2**0 CONTENTS, READONLY, DEBUGGING 34 .debug_frame 00001230 0000000000000000 0000000000000000 0c8d6000 2**3 CONTENTS, READONLY, DEBUGGING 35 .debug_str 0258ab85 0000000000000000 0000000000000000 0c8d7230 2**0 CONTENTS, READONLY, DEBUGGING 36 .debug_loc 06240092 0000000000000000 0000000000000000 0ee61db5 2**0 CONTENTS, READONLY, DEBUGGING 37 .debug_macinfo 00000855 0000000000000000 0000000000000000 150a1e47 2**0 CONTENTS, READONLY, DEBUGGING 38 .debug_pubtypes 02ba7a4e 0000000000000000 0000000000000000 150a269c 2**0 CONTENTS, READONLY, DEBUGGING 39 .debug_ranges 021377a0 0000000000000000 0000000000000000 17c4a0ea 2**0 CONTENTS, READONLY, DEBUGGING // -g ... omitted ... 29 .debug_aranges 00000270 0000000000000000 0000000000000000 018f2294 2**0 CONTENTS, READONLY, DEBUGGING 30 .debug_pubnames 01b582ce 0000000000000000 0000000000000000 018f2504 2**0 CONTENTS, READONLY, DEBUGGING 31 .debug_info 05ae5ecb 0000000000000000 0000000000000000 0344a7d2 2**0 CONTENTS, READONLY, DEBUGGING 32 .debug_abbrev 0014d4d9 0000000000000000 0000000000000000 08f3069d 2**0 CONTENTS, READONLY, DEBUGGING 33 .debug_line 010586d0 0000000000000000 0000000000000000 0907db76 2**0 CONTENTS, READONLY, DEBUGGING 34 .debug_frame 00001230 0000000000000000 0000000000000000 0a0d6248 2**3 CONTENTS, READONLY, DEBUGGING 35 .debug_str 01ca527f 0000000000000000 0000000000000000 0a0d7478 2**0 CONTENTS, READONLY, DEBUGGING 36 .debug_loc 05771c55 0000000000000000 0000000000000000 0bd7c6f7 2**0 CONTENTS, READONLY, DEBUGGING 37 .debug_macinfo 0000078b 0000000000000000 0000000000000000 114ee34c 2**0 CONTENTS, READONLY, DEBUGGING 38 .debug_pubtypes 025cd42a 0000000000000000 0000000000000000 114eead7 2**0 CONTENTS, READONLY, DEBUGGING 39 .debug_ranges 01e419c0 0000000000000000 0000000000000000 13abbf01 2**0 CONTENTS, READONLY, DEBUGGING </pre> </details> Co-authored-by: Boyu Yang <yangby@cryptape.com>
This is something l look very much forward to! Is there an estimate of when this will become stable? |
The performance difference has shrunk substantially adding inline to a bunch of functions. The performance difference with or without lto is about 4 seconds on the slowest clip/qp on a standard awcy run (MINECRAFT, objective-1-fast, qp 80). The compile time difference is 42 seconds. If it's possible to have good performance with lto off, then optimizing without it will provide a good development profile. Going forward, it would be advisable to use lto to check for performancem problems. It may be possible to create a seperate profile between dev and release. Would require rust-lang/cargo#6988 and AWCY would need to be modified to use the new profile/still work with old versions of rav1e.
The performance difference has shrunk substantially adding inline to a bunch of functions. The performance difference with or without lto is about 4 seconds on the slowest clip/qp on a standard awcy run (MINECRAFT, objective-1-fast, qp 80). The compile time difference is 42 seconds. If it's possible to have good performance with lto off, then optimizing without it will provide a good development profile. Going forward, it would be advisable to use lto to check for performancem problems. It may be possible to create a seperate profile between dev and release. Would require rust-lang/cargo#6988 and AWCY would need to be modified to use the new profile/still work with old versions of rav1e.
Is there something stalling this feature? |
@lu-zero I have updated the issue text with some things that need to be done. Someone needs to do the work to finish the implementation, and work on making decisions for some of the design issues. Also, I have not heard of anyone using this, so I do not know if it has gotten any testing. |
👋 used this for profiling with |
Fix LTO with doctests. This fixes an issue where `cargo test --release` would fail to run doctests if LTO is set in `profile.release` or `profile.bench`. The issue is that dependencies were built with `-Clinker-plugin-lto`, but the final rustdoc invocation did not issue `-C lto`. This causes a link failure (or crash!) because the necessary object code was missing. This is because rustdoc historically did not support codegen flags, so Cargo has never passed them in. Rustdoc now supports codegen flags (via rust-lang/rust#63827), so it should be safe to start passing them in. For now, I am only adding LTO, but more should be added in the future. There are two bugs here. One is that LTO flags aren't passed to rustdoc. The other is that the "doctest" unit was using the wrong profile (it was using dev/release when it should be using test/bench). There are two distinct scenarios here. One where just `release` has `lto` set. And one where both `release` and `bench` have `lto` set. This is relevant because LTO mostly cares about what the final artifact wants, and in the case where `bench` does not have `lto` set, then LTO will not be used for tests. This will hopefully be a little cleaner in the future when #6988 is stabilized, which causes the test/bench profiles to *inherit* from the dev/bench profiles, which means you won't need to manually synchronize the test/bench profiles with dev/release. Fixes #8654
Fix LTO with doctests. This fixes an issue where `cargo test --release` would fail to run doctests if LTO is set in `profile.release` or `profile.bench`. The issue is that dependencies were built with `-Clinker-plugin-lto`, but the final rustdoc invocation did not issue `-C lto`. This causes a link failure (or crash!) because the necessary object code was missing. This is because rustdoc historically did not support codegen flags, so Cargo has never passed them in. Rustdoc now supports codegen flags (via rust-lang/rust#63827), so it should be safe to start passing them in. For now, I am only adding LTO, but more should be added in the future. There are two bugs here. One is that LTO flags aren't passed to rustdoc. The other is that the "doctest" unit was using the wrong profile (it was using dev/release when it should be using test/bench). There are two distinct scenarios here. One where just `release` has `lto` set. And one where both `release` and `bench` have `lto` set. This is relevant because LTO mostly cares about what the final artifact wants, and in the case where `bench` does not have `lto` set, then LTO will not be used for tests. This will hopefully be a little cleaner in the future when rust-lang#6988 is stabilized, which causes the test/bench profiles to *inherit* from the dev/bench profiles, which means you won't need to manually synchronize the test/bench profiles with dev/release. Fixes rust-lang#8654
Just to point out another use-case, we want to enable |
This doesn't actually work without some seriously hacky workarounds. A proper solution involves placing everything in one cargo workspace. That won't be practical until custom profiles are available (since the native library/binary require panic=unwind and the rel requires panic=abort) (see rust-lang/cargo#6988 for progress on that)
Posted #9685 with some updates to move this towards stabilization. |
The goal of this change is to support explicitly building particular Cargo profiles per Android build type. This change makes it possible to build both release and debug tasks in a single gradle invocation without editing the use of the `cargo` extension. There are some backwards incompatible changes present: `profile` is deprecated and is replaced with the *required* map `buildTypeToProfile`. This map controls which `cargoBuild${buildType}` tasks are created and what Cargo profile is used for each. Once rust-lang/cargo#6988 is resolved and stabilized, we should switch the implementation to use `cargo build --profile=$x` explicitly rather than `--release`.
The goal of this change is to support explicitly building particular Cargo profiles per Android build type. This change makes it possible to build both release and debug tasks in a single gradle invocation without editing the use of the `cargo` extension. There are some backwards incompatible changes present: `profile` is deprecated and is replaced with the *required* map `buildTypeToProfile`. This map controls which `cargoBuild${buildType}` tasks are created and what Cargo profile is used for each. Once rust-lang/cargo#6988 is resolved and stabilized, we should switch the implementation to use `cargo build --profile=$x` explicitly rather than `--release`.
The goal of this change is to support explicitly building particular Cargo profiles per Android build type. This change makes it possible to build both release and debug tasks in a single gradle invocation without editing the use of the `cargo` extension. There are some backwards incompatible changes present: `profile` is deprecated and is replaced with the *required* map `buildTypeToProfile`. This map controls which `cargoBuild${buildType}` tasks are created and what Cargo profile is used for each. Once rust-lang/cargo#6988 is resolved and stabilized, we should switch the implementation to use `cargo build --profile=$x` explicitly rather than `--release`.
The goal of this change is to support explicitly building particular Cargo profiles per Android build type. This change makes it possible to build both release and debug tasks in a single gradle invocation without editing the use of the `cargo` extension. There are some backwards incompatible changes present: `profile` is deprecated and is replaced with the *required* map `buildTypeToProfile`. This map controls which `cargoBuild${buildType}` tasks are created and what Cargo profile is used for each. Once rust-lang/cargo#6988 is resolved and stabilized, we should switch the implementation to use `cargo build --profile=$x` explicitly rather than `--release`.
To give some measure to the level of confusion: I am trying to debug an issue which depends on a particular opt-level. I have a test for it, and run it with I feel pretty strongly that we should go forward with #2085 and make (via edition or Cargo.toml opt-in) EDIT: the problem I was solving turned out to be pretty cursed -- turns out my bug was depending on lto specifically, and, when two profiles are active ( |
The goal of this change is to support explicitly building particular Cargo profiles per Android build type. This change makes it possible to build both release and debug tasks in a single gradle invocation without editing the use of the `cargo` extension. There are some backwards incompatible changes present: `profile` is deprecated and is replaced with the *required* map `buildTypeToProfile`. This map controls which `cargoBuild${buildType}` tasks are created and what Cargo profile is used for each. Once rust-lang/cargo#6988 is resolved and stabilized, we should switch the implementation to use `cargo build --profile=$x` explicitly rather than `--release`.
One gotcha I hit with when using this is that the Is that expected behavior? I see above that you guys are considering deprecating the However, we are using that variable in build scripts that need to try to find the What do you recommend that we do here if we also want to use named profiles? Will there be another variable that is defined to equal the subdirectory of |
@garbageslam For now, we do not intend to expose the profile name to build scripts. Per https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script, build scripts are not intended to write or interact with directories outside of its OUT_DIR. |
@ehuss one way that we currently use profiles is, we have Because |
I have posted a proposal to stabilize this in #9943. |
Improves library finding by pretending to compile a c file (dummy.c must exist). Now that rust-lang/cargo#6988 is in stable, use "colcon" as a custom profile name. Closes #2. Use of IDL_PACKAGE_FILTER exemplified in README.md. Closes #3
RFC: rust-lang/rfcs#2678
Implementation: #6989
Docs: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#custom-named-profiles
Summary
Adds user-defined profile names.
Unresolved questions:
inherits
keyword name. Decided it should be fine.For example:
target/
. Shouldthe output directory be also under a different namespace, e.g.
target/custom/release-lto
? Decided to use a flat namespace.test
,bench
, or can we make them obsolete? Will keep them for now.test
andbench
outputs intarget/debug
andtarget/release
? Doing so would save compilation time and space. Deferred for future target layout reorg.dir-name
keyword necessary? Alternatively, we can hand-code the output directory ofbench
andtest
to berelease
anddebug
to keep the current behavior. This may be sufficient until a "global binary cache" feature is implemented, or a per-workspacetarget/.cache
(related discussion). Decided to not exposedir-name
and just leave it as a special-case for the built-in profiles.PROFILE
environment variable in build scripts be deprecated? Currently it only setsDEBUG
orRELEASE
, which isn't really useful. Should guide users to useDEBUG
andOPT_LEVEL
instead. Decided to deprecate PROFILE (via documentation).check
rustc
fix
root
?)?cargo build --all-targets
automatically uses the "test" profile for tests. IIRC, named-profiles changes that behavior. I lean towards the original RFC, where it retains the original behavior, unless--profile
is specified, but I am uncertain. Decided to go with a single profile per cargo invocation.test
inheriting fromdev
is a change in behavior, will that be a problem? Decided it shouldn't be a major issue.The text was updated successfully, but these errors were encountered: