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

Bisect through rollups using perf.rlo's individual PR artifacts #202

Closed
lqd opened this issue Aug 24, 2022 · 6 comments
Closed

Bisect through rollups using perf.rlo's individual PR artifacts #202

lqd opened this issue Aug 24, 2022 · 6 comments

Comments

@lqd
Copy link
Member

lqd commented Aug 24, 2022

Recently, support was added to the perfbot to build the artifacts of all the PRs in a rollup. For example, here we can see each sha1 artifact that was built. This is to help perf triage, by manually enqueuing a perf run for suspicious PRs.

These artifacts could also be used by cargo-bisect-rustc for some bisection cases, as a limited workaround to today's stopping bisection at a rollup altogether. Note however that these artifacts are of the PRs themselves, not the accumulated commits of all previous PRs in the rollup: the artifacts for the 3rd PR in the rollup are only of that PR, and do not contain the 1st and 2nd PR's commits.

This is a limitation compared to fully supporting rollups here, but is a sensible step towards that goal: in my experience, it's often the case, but not always, that single PRs within rollups are the source of ICEs or bugs, rather than a unfortunate combination of some of the PRs in the rollup (rare in practice, as the rolled-up PRs are also quite random in general).

Today, bisecting within a rollup requires manually building rustc at each of the rolled-up PR's merge commit, whereas using these rolled-up PRs artifacts would be faster and simpler to find, what I believe is the more common source of bugs: a single PR.

It would be more informative than authoritative: not finding the regression source using these individual artifacts would mean that it's caused by a >1 PRs combination, and proceeding would still require building rustc (and that is fine and expected).

cc @rylev who asked me to open an issue about this

@ehuss
Copy link
Collaborator

ehuss commented Aug 24, 2022

One issue is getting the SHA hashes of the perf runs. Currently, the only way I can think of is to scan the GitHub comments and parse the one posted by the bot (like rust-lang/rust#100426 (comment)). That seems a little brittle, and accessing the GitHub API can also be an issue since it doesn't run terribly well without a token.

@lqd
Copy link
Member Author

lqd commented Aug 24, 2022

Maybe there can be a perf.rlo API returning the hashes for a given rollup id, as I assume they are stored in the database.

cc @Mark-Simulacrum on this possibility.

@rylev
Copy link
Member

rylev commented Aug 26, 2022

They are unfortunately not currently stored in the database so we'd have to add that (which would be doable).

I've created an issue on rustc-perf to track this: rust-lang/rustc-perf#1427

@Kobzol
Copy link
Contributor

Kobzol commented Aug 28, 2022

I was looking at the code and that's not the only obstacle, we also need to find out if a given SHA or nightly string like 2022-08-28 is a rollup at all. But maybe if we had an API to get the unrolled commits, we could just ask it and we wouldn't have to go to GitHub API to find the corresponding PR, see if it's a rollup etc.

@lqd
Copy link
Member Author

lqd commented Sep 15, 2022

An example of the common use-case tracked by this issue: an ICE appeared in a rollup and having the perf artifacts allowed manual bisection: rust-lang/rust#101844 (comment)

@lqd
Copy link
Member Author

lqd commented Feb 11, 2023

Fixed by #256

@lqd lqd closed this as completed Feb 11, 2023
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

No branches or pull requests

4 participants