Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Missing start commit for 90891 #1163

Closed
bjorn3 opened this issue Jan 31, 2022 · 6 comments · Fixed by #1164
Closed

Missing start commit for 90891 #1163

bjorn3 opened this issue Jan 31, 2022 · 6 comments · Fixed by #1164
Assignees

Comments

@bjorn3
Copy link
Member

bjorn3 commented Jan 31, 2022

rust-lang/rust#90891 (comment)

@Mark-Simulacrum
Copy link
Member

It's in the queue - https://perf.rust-lang.org/status.html - I'm not sure why we benchmarked in the wrong order yet. My suspicion is that the PR result was incorrectly queued as a try build, which would prioritize it over master commits, though it would typically have the parent commit still precede it.

@bjorn3
Copy link
Member Author

bjorn3 commented Jan 31, 2022

Should I close this issue then?

@klensy
Copy link
Contributor

klensy commented Jan 31, 2022

1/30/2022, 11:57:34 PM | 08df8b81d6e72 | #92711 - in progress
1/31/2022, 7:23:21 AM | bb549e5afe8f2 | #93270
1/31/2022, 11:12:10 AM | 415c9f95884ca | #93499

No, it's not in queue? I mean, 90891, as it already failed, so other queued after 92711 will fail too.

@Mark-Simulacrum
Copy link
Member

The parent commit (rust-lang/rust@08df8b8) of #90891's merge (rust-lang/rust@e58e7b1) is in the queue -- currently in progress.

@klensy
Copy link
Contributor

klensy commented Jan 31, 2022

I thought, that if no parent for perf run exist, no benchmarks will be run at all, but i was wrong. It will run, but simply don't do comparison.
Perf run results for #90891 exist, so yes, upcoming runs should work :-)

@Mark-Simulacrum
Copy link
Member

OK, I think I likely know what happened:

I think it should be a relatively straightforward thing to change the queue code to fix this -- might actually represent a simplification since I think it'll probably mean dropping the prioritization logic.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants