No profiles generated

[VorpalBlade]

$ cargo init
$ cargo +nightly pgo build
[2024-05-07T19:23:13Z INFO  cargo_pgo::pgo::instrument] PGO profile directory will be cleared.
[2024-05-07T19:23:13Z INFO  cargo_pgo::pgo::instrument] PGO profiles will be stored into /home/arvid/tmp/pgo-reproducer/target/pgo-profiles.
   Compiling pgo-reproducer v0.1.0 (/home/arvid/tmp/pgo-reproducer)
    Finished `release` profile [optimized] target(s) in 0.24s
[2024-05-07T19:23:14Z INFO  cargo_pgo::pgo::instrument] PGO-instrumented binary pgo-reproducer built successfully.
[2024-05-07T19:23:14Z INFO  cargo_pgo::pgo::instrument] Now run /home/arvid/tmp/pgo-reproducer/target/x86_64-unknown-linux-gnu/release/pgo-reproducer on your workload.
    If your program creates multiple processes or you will execute it multiple times in parallel, consider running it with the following environment variable to have more precise profiles:
    LLVM_PROFILE_FILE=/home/arvid/tmp/pgo-reproducer/target/pgo-profiles/pgo-reproducer_%m_%p.profraw
[2024-05-07T19:23:14Z INFO  cargo_pgo::pgo::instrument] PGO instrumentation build finished successfully.
$ ./target/x86_64-unknown-linux-gnu/release/pgo-reproducer
Hello, world!
$  cargo +nightly pgo optimize
[2024-05-07T19:23:46Z WARN  cargo_pgo::pgo::env] llvm-profdata was resolved from PATH. Make sure that its version is compatible with rustc! If not, run `rustup component add llvm-tools-preview`.
No profile files were found at /home/arvid/tmp/pgo-reproducer/target/pgo-profiles. Did you execute your instrumented program?
$ cargo +nightly --version   
cargo 1.80.0-nightly (05364cb2f 2024-05-03)
$ rustup component add llvm-tools-preview
info: component 'llvm-tools' for target 'x86_64-unknown-linux-gnu' is up to date
$ rustup +nightly component add llvm-tools-preview
info: component 'llvm-tools' for target 'x86_64-unknown-linux-gnu' is up to date

This also happens on a much larger program, but I thought a small test case that didn't depend on running on either Arch Linux or Debian specifically would be useful.

The reason I'm trying to use nightly is that I want to use this together with cargo-remark, which complains if I don't use nightly. But it just doesn't seem to work. But the same seems to happen on stable 1.78.

System info:

• rustup 1.27.1 (2024-05-07)
• Rust 1.78 and 1.80 nightly (doesn't make a difference)
• Arch Linux (x86-64)
• cargo-pgo 0.2.8

$ cargo pgo info
[rustc version]: 1.78.0 is recent enough
[llvm-profdata]: found at /home/arvid/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-profdata
[llvm-bolt]: found at /usr/bin/llvm-bolt
[merge-fdata]: found at /usr/bin/merge-fdata

[Kobzol]

Hi, could you please share the reproducer program? Also, does it work when you don't use cargo-pgo, but just use the PGO flags directly?

[VorpalBlade]

The reproducer is any program, as demonstrated by doing cargo init / cargo new and just building the default hello world program.

I will try the flags after work tomorrow.

[Kobzol]

I see. It works for me, so it might be some environment thing.

Did any files appear in target/pgo-profiles? [2024-05-07T19:23:46Z WARN cargo_pgo::pgo::env] llvm-profdata was resolved from PATH. Make sure that its version is compatible with rustc! If not, run rustup component add llvm-tools-preview. sounds a bit suspicious, but if no files appear in pgo-profiles, it's probably caused by something else.

[VorpalBlade]

No files appeared in target/pgo-profiles, I looked at that. It is presumably some environmental thing. I'm writing from my phone at the moment (it is late here), but I do know I use clang+mold for linking and sccache for compiling. Never had any issues with other cargo addons with those, but perhaps cargo-pgo is doing something really weird?

[Kobzol]

cargo-pgo essentially just uses the PGO flags for you, but it is possible that the use linker could be a problem. Once you're able to try the flags directly, we should know more.

[VorpalBlade]

~/.cargo/config.toml:

[build]
rustc-wrapper = "/usr/bin/sccache"
[target.x86_64-unknown-linux-gnu]
linker = "/usr/bin/clang"
# If I comment out the following line, cargo-pgo starts working (traces start showing up in target/pgo-profiles)
rustflags = ["-C", "link-arg=--ld-path=/usr/bin/mold"]

Strangely if I follow the following manual steps from the page you linked:

# STEP 0: Make sure there is no left-over profiling data from previous runs
rm -rf /tmp/pgo-data

# STEP 1: Build the instrumented binaries
RUSTFLAGS="-Cprofile-generate=/tmp/pgo-data" \
    cargo build --release --target=x86_64-unknown-linux-gnu

# STEP 2: Run the instrumented binaries with some typical data
target/x86_64-unknown-linux-gnu/release/pgo-reproducer

Then I do get profiles in /tmp/pgo-data. That is strange. Yes I tripple checked I re-added that rustflags flag to use mold.

[VorpalBlade]

Because maybe it is useful to debug this:

❯ mold --version     
mold 2.31.0 (20fa8d56f5e0c47d1f4bbf7b829c12d3f43298e1; compatible with GNU ld)
❯ clang --version          
clang version 17.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

[Kobzol]

Ah, this might be the weird interaction of trying to combine multiple sources of RUSTFLAGS (#49). I'll try to take a look later.

[Kobzol]

Right, so the issue is that cargo pgo appends to build.rustflags, but that is actually overridden by target.x86_64-unknown-linux-gnu.rustflags. If you move mold to build.rustflags, it should work.

[VorpalBlade]

Right, so the issue is that cargo pgo appends to build.rustflags, but that is actually overridden by target.x86_64-unknown-linux-gnu.rustflags. If you move mold to build.rustflags, it should work.

I seem to remember I did that to fix some issue involving cross compilation with cross-rs. Or maybe it was embedded cross compiling to ESP32 (I have done both).

That said, I would expect such flags to be appending rather than over writing, that is a bit of a footgun.

[Kobzol]

Yeah, that's sadly a cargo issue (rust-lang/cargo#5376).

[danbulant]

Having similar issue.
In my case, even removing rustflags (and clearing target) completely from Cargo.toml didn't solve the it and I had to run it manually.
I do have changed options in the release profile, maybe that also can change things?

[profile.release]
codegen-units = 1
lto = "fat"
panic = "abort"
strip = false
debug = 1

[Kobzol]

Do you perhaps have a config.toml file in some of the parent directories, or in your home directory? It can also be applied from there.

[jonasmalacofilho]

Since --target is already passed to the build

cargo-pgo/src/build.rs

Lines 157 to 167 in 4f69cbb

	// --target is passed to avoid instrumenting build scripts
	// See https://doc.rust-lang.org/rustc/profile-guided-optimization.html#a-complete-cargo-workflow
	if !parsed_args.contains_target {
	let default_target = get_default_target().map_err(|error| {
	anyhow::anyhow!(
	"Unable to find default target triple for your platform: {:?}",
	error
	)
	})?;
	command.args(["--target", &default_target]);
	}

couldn't the PGO flags be passed as target.*.rustflags as well?

Since linker flags are target dependent, moving them to build.rustflags is not ideal, especially for those that regularly compile to different targets.

[Kobzol]

Ah, that's a good point! I thought that it would just override the target rustflags, but it actually appends to them, so that could work. I'll try to take a look at it once I have a bit of time.

[jonasmalacofilho]

I'm not sure what's preferable, the specific target or the target.'cfg(all())'.rustflags trick?

With the latter things work almost just as before, so this patch is enough:

diff --git a/src/build.rs b/src/build.rs
index 7daf3016a06e..f8765eec2511 100644
--- a/src/build.rs
+++ b/src/build.rs
@@ -93,10 +93,11 @@ pub fn cargo_command_with_rustflags(
         (true, _) => {
             // If there's no RUSTFLAGS, and Cargo supports `--config`, use it.
             // The user might have some flags in their config.toml file(s), and
-            // `--config build.rustflags` will append to it instead of overwriting it.
+            // `--config target.*.rustflags` will append to it instead of overwriting it.
             final_cargo_args.push("--config".to_string());
 
-            let mut flags = String::from("build.rustflags=[");
+            // `cfg(all())` always matches the current target.
+            let mut flags = String::from("target.'cfg(all())'.rustflags=[");
             for (index, flag) in rustflags.into_iter().enumerate() {
                 if index > 0 {
                     flags.push(',');

But maybe specifying the target would be more understandable, given that's needed anyway when setting --target in cargo_command?

[Kobzol]

That's a cool trick, I didn't know about that. I think that cfg(all()) is better, because you can compile for multiple targets at once (cargo build --target T1 --target T2 etc.), and that would be more difficult to handle with per-target options. Thanks for the patch, I'll try to write a few tests for this.

[Kobzol]

Ok, it seems to work. But, of course, this only moves the issue to a different place. Because now if you have build.rustflags in your config.toml file, it will get overridden by the target rustflags. So we can select to either discard build rustflags or target rustflags :)

Either target flags override PGO, or PGO overrides build flags. But maybe target flags are used more commonly, so it makes sense?

[Kobzol]

• https://github.com/search?type=code&auto_enroll=true&q=path%3A.cargo%2Fconfig.toml+%5Bbuild%5D - 7.3k
• https://github.com/search?type=code&auto_enroll=true&q=path%3A.cargo%2Fconfig.toml+%5Btarget - 7.1k

Seems to be about the same, although this doesn't check rustflags usage.

I guess it might make more sense for cargo pgo to "just work", even if it discards some flags, rather than silently failing to produce PGO profiles.

[Kobzol]

Can you try if this PR works for you?

$ cargo install --git https://github.com/kobzol/cargo-pgo --branch use-target-rustflags

[jonasmalacofilho]

Ok, it seems to work. But, of course, this only moves the issue to a different place. Because now if you have build.rustflags in your config.toml file, it will get overridden by the target rustflags. So we can select to either discard build rustflags or target rustflags :)

Yeap : )

Either target flags override PGO, or PGO overrides build flags. But maybe target flags are used more commonly, so it makes sense?

I would expect target rustflags to be more common, but some people may prefer build rustflags (even when target rustflags may otherwise be a better choice) because they are simpler to set up, especially without the cfg(all()) trick.

Perhaps defaulting to target rustflags, with an option (command-line or environment) to switch build rustflags?

---

I'll try the PR in a bit, thanks!