Heap flamegraph: sort roots by name to improve stability #3911
Conversation
🎨 Perfetto UI Build✅ UI build is ready: https://storage.googleapis.com/perfetto-ci-artifacts/gh-19829868466-ui/ui/index.html |
lukaville
changed the title
| } | ||
|
|
||
| std::unique_ptr<tables::ExperimentalFlamegraphTable> | ||
| HeapGraphTracker::BuildFlamegraph(const int64_t current_ts, |
There was a problem hiding this comment.
This seems like the wrog place to do this. Sure it will work for pprof but then pprofs will not be consistent with the Perfetto UI.
Also experimental_flamegraph is intended for deletion with the pprof export code being rebased on top of the same code the UI uses: at that point again this will break.
There was a problem hiding this comment.
Ah I thought experimental_flamegraph is also used in the UI. Would it make sense to introduce similar change to both experimental_flamegraph and whatever the UI currently uses?
Seems like currenlty UI uses _heap_graph_object_min_depth_tree for this?
There was a problem hiding this comment.
Yes _heap_graph_object_min_depth_tree is what you would need to modify.
2 participants
Currently often the GC root could be chosen non-deterministically (based on the row number) when calculating shortest path to the GC root in java heap flamegraph table.
Sorting them by name could improve stability for the cases when we want to compare heap dumps. We might consider sorting all sibling nodes by name when calculating path to roots, but currently it seems like root instability causes the biggest problem. Added only sorting of the roots in this PR to ensure minimum impact on performance.
E.g. in these two traces
dalvik.system.PathClassLoaderhas exactly the same set of GC roots that reference it but we chose different roots, this causes issues when trying to compare the trees (they are treated as completely separate subtrees):