Skip to content
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

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

Closed
zamazan4ik opened this issue Sep 22, 2024 · 1 comment · Fixed by #61
Closed

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

zamazan4ik opened this issue Sep 22, 2024 · 1 comment · Fixed by #61

Comments

@zamazan4ik
Copy link
Contributor

Hi!

In WGUNDERWOOD/tex-fmt#22 (comment) 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
Copy link
Owner

Kobzol commented Sep 22, 2024

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.

zamazan4ik added a commit to zamazan4ik/cargo-pgo that referenced this issue Oct 1, 2024
@Kobzol Kobzol closed this as completed in #61 Oct 1, 2024
@Kobzol Kobzol closed this as completed in 71de657 Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants