Add graphs for memory usage comparison

Shopify / yjit-metrics

"Tasks for benchmarking, building and collecting stats for YJIT"

MIT License

14 stars 9 forks source link

Add graphs for memory usage comparison #267

Closed rwstauner closed 5 months ago

rwstauner commented 5 months ago

Make speedup SVG generation code more generic to enable future reuse
Remove unnecessary attr_reader
Remove unused method override and instance variable
Prepare SpeedDetails (single and multi) report classes for subclassing
Produce memory based graphs like we have for speed
Skip stddev bars when the stddev is zero
Include memory graphs after the speed graphs

I intend to refactor the classes out to their own files and abstract methods, etc, but I had already done a bunch of renames and I'd like to keep the refactor separate from any feature changes.

I added the corresponding memory graph below each place where we showed the speed graphs

Currently the description is below the graph but I think it might be better above I'm also thinking about maybe adding a horizontal line between the graphs or something to separate them a little more visually. What do you think?

maximecb commented 5 months ago

The memory graph is harder to interpret than I expected (because the bars appear very random), but it looks very good 👍

Currently the description is below the graph but I think it might be better above I'm also thinking about maybe adding a horizontal line between the graphs or something to separate them a little more visually. What do you think?

I think it might be good to have a short title/heading above each graph and a longer description below the graphs like you suggested.

I would like to rename the first heading "Headlining Benchmarks" => "Performance on Headline Benchmarks" And for the second graph we could have "Memory Usage on Headline Benchmarks".

In the description of the performance graphs, we should remove the mention of "iterations/second" because that doesn't correspond to the units shown on the graph. We could mention that every bar is normalized such that the CRuby 3.3.1 interpreter has value 1.0.

rwstauner commented 5 months ago

In the description of the performance graphs, we should remove the mention of "iterations/second" because that doesn't correspond to the units shown on the graph. We could mention that every bar is normalized such that the CRuby 3.3.1 interpreter has value 1.0.

Thanks, yeah, when I added the memory graph I had put "(in mebibytes)" to mirror the first one, then realized that you couldn't tell what unit the original measurement was anymore so I took that out.

rwstauner commented 5 months ago

I made the header and descriptions more consistent

maximecb commented 5 months ago

Beautiful