Kobzol / cargo-pgo

Cargo subcommand for optimizing Rust binaries/libraries with PGO and BOLT.
MIT License
573 stars 11 forks source link

options for older OS #35

Closed lidh15 closed 1 year ago

lidh15 commented 1 year ago

I'm using cargo-pgo on CentOS 7 following the instruction in README, when I ran cargo pgo build, undefined reference to memfd_create in memfd.rs was thrown. I googled and it's said that this is due to a too-old glibc, so are there any suggestions for users on these OS with only glibc2.17, or the optimization is based on a newer OS so we have to update the working environment?

Kobzol commented 1 year ago

Hi, and regular cargo build works for you? Rust should support glibc 2.17+ (https://blog.rust-lang.org/2022/08/01/Increasing-glibc-kernel-requirements.html), CentOS 7 is actually used to produce the Linux Rust distribution artifacts, so it should be working on that system.

lidh15 commented 1 year ago

regular cargo build works

Kobzol commented 1 year ago

Interesting. Could you send me the whole error please?

lidh15 commented 1 year ago

I cannot copy the whole error, I'll try to describe it. it is linking with 'cc' failed: exit status: 18 and the note said that the error is in function nix::sys::memfd::memfd_create at nix-0.26.2/src/sys/memfd.rs:56: undefined reference to memfd_create the cargo pgo backtrace: a044b9f2cdcd82d94f761aef0780fb8

lidh15 commented 1 year ago

well, I found some clue here, is it about how I config profile in Cargo.toml ?

lidh15 commented 1 year ago

well, I found some clue here, is it about how I config profile in Cargo.toml ?

this commit works for me, it seems that the error is not related to cargo-pgo and is only about nix, I'm not sure will they merge this commit into release.

lidh15 commented 1 year ago

now at the bolt build step after cargo pgo bolt build --with-pgo, it throws error linking with cc failed: exit status: 11, /opt/rh/devtoolset-11/root/usr/libexec/gcc/x86_64-redhat-linux/11/ld: final link failed: invalid operation. I googled but still had no idea about this error. Some said that invalid operation means that I'm using not supported CPU instruction set, but I'm doing everything on the same machine and it should be native. Do you have any clues for this error?

Kobzol commented 1 year ago

I saw this error when I had enabled stripping in Cargo.toml (cargo-pgo should probably detect this and warn about it). Do you have stripping enabled?

lidh15 commented 1 year ago

I saw this error when I had enabled stripping in Cargo.toml (cargo-pgo should probably detect this and warn about it). Do you have stripping enabled?

okay, it seems so

lidh15 commented 1 year ago

And I still have some questions, when I run my -bolt-instrumented or -bolt-optimized binary, the program reported assertion failed: failed to get folder entries after exit and there is no file in target/bolt-profiles/<binary>/. As a result, I think, the bolt optimize warned: No profiles found for target <binary>. I'm using a non-root user and all the folders belong to the correct user and user group. I wonder who forbids cargo pgo out of the folder? How should I modify my workflow?

Kobzol commented 1 year ago

I haven't seen this error yet. Maybe you could try running your -bolt-instrumented binary with strace and try to see what errors are returned by the kernel when the program tries to access this directory?

lidh15 commented 1 year ago

I haven't seen this error yet. Maybe you could try running your -bolt-instrumented binary with strace and try to see what errors are returned by the kernel when the program tries to access this directory?

I traced it and saw following report:

…
open("/proc/self/map_files/", 0, 438) = 3
getdents(3, 0x7FFC6D2FC290, 1024) = -1
write(2, "Assertion failed: failed to get folder entries", 47) = 47
exit_group(1) = -38

it seems that it failed to write /proc/self/map_files/? I googled and it is said to be a symbol link to /proc/pid/map_files/, but there is no reason why it is not allowed to be writen.

Kobzol commented 1 year ago

It seems that it fails to mmap some file, I have no idea why it fails with an error "no such process" though. I have asked on the LLVM Discord about this.

rafaelauler commented 1 year ago

Try using option -instrumentation-binpath=PATH where PATH is the full path to the instrumented binary. That's the option description:

"path to instumented binary in case if /proc/self/map_files is not accessible due to access restriction issues"

More details on this problem: the BOLT instrumented version of a binary, shortly before writing the profile to disk, will try to open itself (the ELF file) to read instrumentation data structures stored there. It will discover which ELF file to read by figuring out what is the file backing the current PC address. This can be done by reading the files in the folder /proc/self/map_files. It's not clear to me why, but apparently some systems may have permission issues and won't show you the contents of this folder. In this case, there's a workaround: you can still manually provide the binary path using the option above.

lidh15 commented 1 year ago

I didn't find how to pass it to llvm-bolt, should I do cargo pgo bolt build --with-pgo -- --instrumentation-binpath=PATH?

lidh15 commented 1 year ago

I didn't find how to pass it to llvm-bolt, should I do cargo pgo bolt build --with-pgo -- --instrumentation-binpath=PATH?

Okay, I inserted the option into fn instrument_binary in src/bolt/instrument.rs and it worked.

Kobzol commented 1 year ago

I wonder if cargo-pgo should just do this by default.

@rafaelauler Assume that we're controlling the llvm-bolt execution and we know the path to the instrumented binary. Is there any disadvantage to always setting --instrumentation-binpath? I guess that if someone moves the binary, it would break, but other than that?

rafaelauler commented 1 year ago

I guess there are no other disadvantages, other than not allowing the binary to be moved.

Kobzol commented 1 year ago

I see, thank you. That might be tricky for users though, so I will probably just leave it as it is for now. People who encounter this problem can hopefully find this issue.

Btw @lidh15 You can pass the args without modifying the source code like this:

$ cargo pgo bolt build --with-pgo --bolt-args "--instrumentation-binpath=..."

But in this specific case you need to fill in the correct instrumented binary path.

lidh15 commented 1 year ago

I see, thank you. That might be tricky for users though, so I will probably just leave it as it is for now. People who encounter this problem can hopefully find this issue.

Btw @lidh15 You can pass the args without modifying the source code like this:

$ cargo pgo bolt build --with-pgo --bolt-args "--instrumentation-binpath=..."

But in this specific case you need to fill in the correct instrumented binary path.

Thank you