ocurrent / current-bench

Experimental benchmarking infrastructure using OCurrent pipelines
Apache License 2.0
34 stars 17 forks source link

Allow clicking on the legend to show/hide plot lines and use more columns #323

Closed punchagan closed 2 years ago

punchagan commented 2 years ago

Before image

After image

art-w commented 2 years ago

That's a much better default indeed! What do you think of sorting the labels in desc order? And perhaps collapsing the very small categories together?

``` assemble:0.2680secs c-linker:2.891secs cse:0.02300secs deadcode:0.01100secs emit:0.02100secs generate:7.823secs linearize:0.001000secs liveness:0.03950secs parsing:0.6555secs polling:0.003000secs regalloc:0.2265secs scheduling:0.001000secs selection:0.04100secs spill:0.03250secs split:0.01800secs transl:0.5290secs typing:4.412secs ``` ``` generate:7.823secs typing:4.412secs c-linker:2.891secs (12 grouped):0.6855secs parsing:0.6555secs transl:0.5290secs ```
punchagan commented 2 years ago

Great suggestions, thanks!

The sort in descending order of values makes a lot of sense. I think it would also make sense for almost all of the overlaid graphs. There could be some confusion when comparing two versions of the graph (across different branches or some thing like that), when the sort order changes because the values change slightly. But, I don't think this is a common enough case to worry enough about?

The grouping of smaller values as one entry makes sense. I was also wondering if it would make sense to show a total of these also as another "entry", similar to the grouped total of small values.

I'm not sure if the last couple of ideas are generic enough for all benchmarks that we want to add this in the UI, or if we should do this in the ocaml benchmark JSON generation step. Do you think overlaid graphs would be used mostly for this kind of a use case where we are splitting up the total value into smaller values and overlaying them on a single graph? May be we should worry about other use cases when we actually hit them?

art-w commented 2 years ago
punchagan commented 2 years ago
  • Yeah, I also think that sorting makes sense for everybody. The order confusion should be rare... and we'll probably work on simplifying the comparison of different branches at some point!

I've updated the PR to sort the legend in decreasing order of values.

art-w commented 2 years ago

Many thanks, it will really help for the ocaml compiler graphs!