Compare page: Include current run into 30day history even if it's not merged

[panstromek]

I'm thinking of something like

I find the 30day history graph incredibly useful to quickly check how the current run compares to usual benchmark behaviour. I use it all the time during triage.

Sadly you can't do this quick check when the PR is not merged or when the run is on PR from merged rollup. At that point, you have to eyeball the comparison between the percentage number and changes in the graph, which is a bit awkward - especially since you can't see where is the parent commit in that graph by default.

[Kobzol]

Hi, I'm not sure if I understand. The 30-day history graph always ends with the commit being compared (regardless whether it's a master or a try commit). So the "current run" is always at the top right.

[panstromek]

hmm, Is that really the case? When I click through this for example: https://perf.rust-lang.org/compare.html?start=2300c2aef7dbc2a7bbbeaa9894d07d459abd9bc6&end=701ca7229fdda54e3b2aaf5d347786d78565ec43&stat=instructions:u, the commit being benchmarked is 701ca722 but the graph ends with 2300c2ae (which is the parent):

[Kobzol]

Sorry, I confused myself :) There is indeed a difference between try/master builds; what I described is true for try builds.

• For master builds, we show a 30 day period around them, so that we can see the trends (https://perf.rust-lang.org/compare.html?start=442288534b6cf9ec4899b00c4332433b17760d96&end=360a3a4e65cb70ea6e9da69cbb15f460dee516fd&stat=instructions:u).
• For try builds, we show the 30 day period up to the try commit.

I think that I was confused by your visualization, because a try build won't ever currently be displayed "in the middle" of the 30 day period:

So I think that we can make two changes:

1. Include the try build in the chart at the rightmost position, which should show the change from its parent commit (although that is visible also from the table itself)
2. Treat try builds the same as master builds, and show a 30 day history around them, so that you can also see future results when you examine a try build.

Did you mean 2)?

[panstromek]

I meant both (kindof), actually, sorry for confusion.

1. Including try build into the graph - yes, that's the main idea

2. Not sure if "treat the same" means what I think it means, but definitely showing 30 day history around them. Which in some cases could look like the the visualisation - specifically, this case could happen when you measure a single PR from merged rollup. The orange track would be history on the main branch, while the green point would show "what would happen if this PR was merged individually". In other words, that try build would share the same column in the graph as the rollup it was originally a part of.

Apart from the rollup case, I also find it useful to see this for try builds to notice spurious changes. e.g. when bimodal benchmarks returns to default state on your try build, but you see it also returned to default state on master, you can guess it's probably irrelevant to your PR.

[Kobzol]

Okay, yeah, I can see that being useful. It might be tricky to implement though, because the current code looks at the chain of commits on master, and we would somehow need to implant the try commit into that at the "right" place.

The try build would have to be implanted somewhere around here: https://github.com/rust-lang/rustc-perf/blob/dea9f1e089caef953c1e6ff4987d1f7f167da99c/site/src/request_handlers/graph.rs#L30