-
Notifications
You must be signed in to change notification settings - Fork 152
Switch fraction chart back to absolute values and improve its legend #1806
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
Conversation
I don't feel that at all. I strongly prefer the absolute view and find the relative view quite misleading -- with the relative view I immediately think "why did the frontend get slower and the linker faster?" which is quite wrong. You can use the percentages in the text label to get the relative values. I won't object to having both if you find the relative view useful, but I think absolute should be the default. |
I guess it's quite subjective, yeah. Before, we had the absolute view, and @lqd found it quite confusing and we converged on the relative view 😆 |
My recollection is that it wasn’t absolute before, and there were no time values only relative percentages, and that the visualization changed total size sometimes on the before row, sometimes on the after row. So we were displaying relative percentages of a different total time, making the values incomparable. I also had mentioned a case where I wished to see absolute numbers. So I wasn’t able to compare how much of the process a section took nor the absolute time taken by a section. As I said before, I believe we probably only need the absolute time each section took (albeit noisy wall times), in a table. |
It's possible that the visualization was different before, I changed the code slightly in the meantime. This PR already added the absolute times to the hover labels, btw. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the chart legend could also be displaying the expected units, as it's currently showing the relative values even when the chart is in absolute mode.
site/frontend/src/pages/compare/compile/table/benchmark-detail.vue
Outdated
Show resolved
Hide resolved
It started to overflow when I included more data 🤦♂️ I'll try to tinker with the design a little bit to fix that, the chart probably doesn't need to be so large. |
This also happens with the new "absolute mode". It needs to work like this, otherwise the width of the longer collection could be unbounded (AFAIK). |
76f85ff
to
cf26f96
Compare
Great! |
The fraction chart in benchmark detail on the compare page is designed to show two things:
However, it is quite challenging (perhaps impossible) to display both of these things in a single chart, without skewing the results.
Before #1775, the chart showed 2), which made 1) skewed. After that PR, the chart shows 1), which doesn't really even allow comparing 2).
Here is an example of the skew that was happening before #1775:

In this case, the absolute time of the frontend (purple part) has stayed the same, but its fraction within the collection has changed a lot. But visually, I think that the first thing that my mind when I see it is "the fraction is the same", because the purple rectangle has the same width.
Based on reactions on this PR, it seems that the absolute view is actually OK, and the "skew" isn't problematic for people to undestand. So this PR reverts the display to the absolute value.
Apart from that, the PR also makes tiny stylistic changes in the chart and the legend.