search

Kobzol / cargo-pgo

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

A bit misleading "run your binary for at least a minute or more" wording #60

Closed zamazan4ik closed 1 month ago

zamazan4ik commented 1 month ago

Hi!

In https://github.com/WGUNDERWOOD/tex-fmt/issues/22#issuecomment-2360274325 we found an issue that people are running their PGO training workloads according to the recommendations from the cargo-pgo's README file "for at least a minute". I completely understand your intention why did you write such a recommendation in the README file? However, as we see, this recommendation can be misleading for people - such a recommendation can be at least useless in some cases.

Maybe we can think about changing the wording a bit to something else. Honestly, I don't know what to write instead since choosing the proper PGO training workload is more like an art than a simple step-by-step algorithm. Or we can just remove it at all from the README file :)

Kobzol commented 1 month ago

It's true that the minimum duration recommendation is more applicable when sampling is used, however cargo-pgo uses instrumentation, so that's a bit different indeed. I guess that the main point is to exercise all the important parts of the codebase (in the coverage sense).

Feel free to send a PR that rewords this.