-
Notifications
You must be signed in to change notification settings - Fork 55
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
Comments
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. |
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. |
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 |
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 |
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) |
Fixed by #256 |
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
The text was updated successfully, but these errors were encountered: