search

rust-lang / cargo-bisect-rustc

Bisects rustc, either nightlies or CI artifacts
https://rust-lang.github.io/cargo-bisect-rustc/
Apache License 2.0
174 stars 55 forks source link

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

Closed lqd closed 1 year ago

lqd commented 1 year ago

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 commented 1 year ago

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 https://github.com/rust-lang/rust/pull/100426#issuecomment-1212610788). 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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/rust-lang/rustc-perf/issues/1427

Kobzol commented 1 year ago

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 commented 1 year ago

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: https://github.com/rust-lang/rust/issues/101844#issuecomment-1248309182

lqd commented 1 year ago

Fixed by #256