Add summary table to compare page

[Kobzol]

I moved the compare page JS, CSS and HTML to individual pages in #1400 to make it easier to modify the compare page. It's still pretty annoying to do large changes, since it's written in JS (Typescript would be really nice!), but I think that we can do some incremental changes.

Inspired by #1335, this PR adds a summary table component that is now used on the compare page instead of the previous all/filtered summary lines. I think that the all/filtered distinction was mostly useless and that it's much simpler to have a single summary + a Reset filters button, which this PR adds.

The new table contains min/max range and the number of unique benchmarks affected. I'm not sure which icon to use for the "all" (last) row, maybe some globe? Or no icon at all.

The first row shows regressions, the second row shows improvements, and the third row shows all changes (regressions + improvements).

As a follow-up, I would like to add a collapsible section with various axis aggregations, e.g. per-scenario, per-profile, per-category, and use this new summary table in the aggregations to easily display the results.

Related issue: #1328

[nnethercote]

I haven't looked closely at the code changes because I don't know that code (or webdev stuff in general) well. But I like the visual change. Especially showing the number of results that didn't change. (Similar to what I tried here. I think it makes sense for this summary and the GitHub comment summary to eventually look the same.)

If you copy and paste the new summary (e.g. into a GitHub comment) does it reproduce nicely?

[nnethercote]

Change the emoji like @rylev said and I think this is good to go, thanks!

[Kobzol]

Especially showing the number of results that didn't change.

I'm a bit confused by this. Actually, the summary line before has showed the number of results that didn't change. Now the table instead shows regressions, improvements and all changes (so number of benchmarks that didn't change is not shown, although it can be calculated relatively easily by taking the third row of the third column and subtracting the first and second rows).

This is how the table looks on GitHub:

Range	Mean	Count
[0.30%, 2.78%]	0.82%	133 (26)	❌
[-0.30%, -26.83%]	-2.48%	269 (44)	✅
[2.78%, -26.83%]	-1.39%	402 (44)

We can't play with CSS or color, but it doesn't look that bad (we could right-align the cells to make them a bit better aligned). Of course it also doesn't contain the category distinction, so it's not a full replacement for the current PR summary table.

[nnethercote]

Actually, the summary line before has showed the number of results that didn't change.

I see. So there's room for confusion there, even among rustc-perf experts. Can you add a label for each row: "Worse", "Better", "All"? (Again, that's similar to what I did in #1365.) It's a shame there isn't an emoji that clearly means "All".

This is how the table looks on GitHub:
Range Mean Count
[0.30%, 2.78%]
0.82% 133 (26) ❌
[-0.30%, -26.83%]
-2.48% 269 (44) ✅
[2.78%, -26.83%]
-1.39% 402 (44)

We need to be careful with the ranges here.

• [0.30%, 2.78%] The best (least regressed) result is listed first.
• [-0.30%, -26.83%] The best (most improved) result is listed second.
• [2.78%, -26.83%] The best (most improved or least regressed) result is listed second.

What I did in #1365 was to always put the smallest (most negative) one first and the largest (most positive) one second. This would change the ordering of the last two rows in the table, giving:

• [0.30%, 2.78%]
• [-26.83%, -0.30%]
• [-26.83%, 2.78%]

[Kobzol]

I see. So there's room for confusion there, even among rustc-perf experts. Can you add a label for each row: "Worse", "Better", "All"? (Again, that's similar to what I did in #1365.) It's a shame there isn't an emoji that clearly means "All".

I was trying to avoid another label column to make the table smaller. It's not a problem in the main table, but then I want to include a lot of small tables for various aggregations (per-profile, per-category etc.). As an attempt to avoid such column, what about this?

Range	Mean	Count
[0.30%, 2.78%]	0.82%	133 (26)	❌
[-0.30%, -26.83%]	-2.48%	269 (44)	✅
[2.78%, -26.83%]	-1.39%	402 (44)	❌+✅

We need to be careful with the ranges here.

I did that on purpose, although I realize that it's a bit controversial. To me it made sense to read it as "[smallest regression, largest regression]" and "[smallest improvement, largest improvement]". But now I realize that this interpretation is not consistent with the third row, where it's "[smallest improvement, largest improvement]", but it's not clear why it's not the opposite order.

Your ordering is more consistent. @rylev what is your opinion about the range ordering?

[nnethercote]

I now realize I don't understand the "Count" column. What does "133 (26)" mean?

"❌+✅" makes me think "regressions and improvements", i.e. excludes ones that didn't change.

[Kobzol]

I now realize I don't understand the "Count" column. What does "133 (26)" mean?

If you hover on it with a mouse, it has a title (I know that you can't do that now :D I just wanted to let you know that there's a hint for the users, although maybe it's not that much discoverable). But I realize that the presentation isn't ideal (I couldn't think of a better one without adding another column though). The number in the parentheses is the number of unique benchmarks changed/affected.

"x+white_check_mark" makes me think "regressions and improvements", i.e. excludes ones that didn't change.

That's exactly what the data contains :) It's regressions + improvements, nothing more, nothing less. To include non-relevant results, you can click on "Show non-relevant results". Other than that, there's not really any other "didn't change" results. Basically, all the results are changes, some are relevant, some are non-relevant (out of the non-relevant ones, some might actually be no change at all, but that is probably quite rare). You can use the checkbox to display or hide these non-relevant results.

Before the summary line was a bit weird, since it also included data about the non-relevant results, and the checkbox was basically just setting the "not changed" counter to 0 or "count of non-relevant results".

[nnethercote]

"x+white_check_mark" makes me think "regressions and improvements", i.e. excludes ones that didn't change.

That's exactly what the data contains :) It's regressions + improvements, nothing more, nothing less. To include non-relevant results, you can click on "Show non-relevant results". Other than that, there's not really any other "didn't change" results.

Above you said:

Now the table instead shows regressions, improvements and all changes (so number of benchmarks that didn't change is not shown, although it can be calculated relatively easily by taking the third row of the third column and subtracting the first and second rows).

So I thought the last row was everything. But I see now that 133 + 269 = 402. I would rather see the range and mean for every result, rather than the range and mean of all the improvements and regressions. The former gives a good idea of the effect across all the benchmarks, while the latter often looks large but that's because only a small number of benchmarks are affected.

[Kobzol]

Sorry, what I said before about the row substraction was completely wrong, I probably also got confused by my table :D The third row is indeed just regressions + improvements, so all data combined.

I wanted to make the table self-contained, so that it describes a specific dataset, without considering any other data (like non-relevant results). I think that this approach is correct and that the previous approach, which mixed information from data that was not presented (non-relevant results) was not intuitive.

The table now has the property that it shows regressions, improvements, and all changes amongst the currently filtered data. So the table summarizes exactly the data that is shown in the "changes" table/list below. With this property, there is a very easy way of displaying changes amongst all benchmarks - just check the "Show non-relevant results" checkbox. I think that this behaviour is much more intuitive, rather than mixing relevant and non-relevant data within the same table.

[nnethercote]

If I click "show non-relevant results" the bottom row of the table will change. What about the other two rows? Will they change?

[Kobzol]

Yes :) The table now always shows a summary of the currently filtered data (benchmark name, scenario, profile, category, relevance). So if you change the relevance filter, it will update the table.

Show non-relevant results: unchecked

<regressions taken from relevant data>
<improvements taken from relevant data>
<all changes taken from relevant data>

Show non-relevant results: checked

<regressions taken from all data>
<improvements taken from all data>
<all changes taken from all data>

[rylev]

what is your opinion about the range ordering?

I agree with @nnethercote that the order should be how you would see the numbers on a number line.

The table now always shows a summary of the currently filtered data (benchmark name, scenario, profile, category, relevance). So if you change the relevance filter, it will update the table.

I think this makes a lot of sense and intuitively makes sense. It summarizes the visible data.

[Kobzol]

I reverted the range order and made a few visual changes based on received feedback. I also tried to align the number in the range so that in most cases the backets ([]) will line up nicely, and put back the plus sign for positive numbers.

[rylev]

Thanks for working on this! I think this is certainly an improvement over the status quo. I'm hesitant to send this back for another round of bike shedding when we can just get it into master and discuss small improvements as follow ups.

[Kobzol]

Let's wait for @nnethercote, if he has more comments.

[nnethercote]

On the last line I'd put the cross first, given that we generally show regressions first.

I like the addition of the + symbol everywhere.

Otherwise, this is good enough for me. Even if it's not perfect it'll be useful to try it out for a while, see how it feels in practice.

[Kobzol]

I agree, let's try it. I squashed the commits.

[lqd]

Suggested change

	<td title="Regresions">❌</td>
	<td title="Regressions">❌</td>

[Kobzol]

Oops :) Thanks. Will fix in #1410 if it gets merged, otherwise I'll send a separate PR.