bloomberg / memray

Memray is a memory profiler for Python
https://bloomberg.github.io/memray/
Apache License 2.0
13.17k stars 392 forks source link

docs: Make flame graph docs reflect icicle default #466

Closed tjmcewan closed 1 year ago

tjmcewan commented 1 year ago

Doc update to align with what appears to be a relatively new default flame graph orientation.

godlygeek commented 1 year ago

Doc update to align with what appears to be a relatively new default flame graph orientation.

You'd think that, but nah - I'm pretty sure we've been using icicle orientation by default since our very first release, and this has been wrong forever 😅

Thanks for catching this! I'm going to go for a more extensive fix than you proposed, though. I've updated the first section to avoid absolute spatial references entirely, and only make references relative to the location of the root. Then, I've yanked up the section about flame graphs vs icicle graphs to right below that section, explaining the flame vs icicle distinction earlier, explaining that the root could appear in either of two positions, and making it clear that memray flamegraph produces icicle graphs by default despite its name. And I've updated the prose for the first of the two simple examples to make it clear that it's a flame graph as opposed to an icicle graph. I didn't bother for the second, since I think it's fair to assume anyone trying to read and interpret the second example flame graph will have already understood the first example.

Since I've made such extensive changes to your small original patch, I'm going to leave this for someone else to review my work.

https://godlygeek.github.io/memray/flamegraph.html shows this page with my proposed changes.