I should be able to list all objects in the heap in tree-form showing from the GC root, a path of object references leading to the object in question. Alternative takes on this from YourKit Java Profiler "Merged Paths" view, that’s similar to path from GC view, but does not show path of individual objects, but shows paths from multiple objects grouped by class. https://www.yourkit.com/docs/java/help/merged_paths.png Chrome's containment view: https://developer.chrome.com/devtools/docs/javascript-memory-profiling#containment-view Can also be inversed, as an "Outgoing References" view http://techpubs.borland.com/optimizeit/optimizeit5/profiler/images/outgoing_win.gif

Fundamental feature of the memory tool. Probably highest priority feature after dominator trees land.

This is very much WIP. I think we need a splitter component so we can resize the shortest paths side bar first...

Stil WIP, but getting closer...

This commit extends the devtools' memory panel's dominators view with a sidebar that displays the 5 shortest retaining paths from the GC roots for the selected node in the dominator tree. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aee612965a2b

Comment on attachment 8720979 [details] [diff] [review] Render retaining paths in the dominators view Review of attachment 8720979 [details] [diff] [review]: ----------------------------------------------------------------- Looks good -- only concern I have is whether or not we want to store the size of the h splitter in our main state, or just have that component handle it on its own. It certainly adds complexity here, and think it should be a view state, not a redux property, but defer to others on that. ::: devtools/client/memory/components/shortest-paths.js @@ +11,5 @@ > +const { isSavedFrame } = require("devtools/shared/DevToolsUtils"); > +const { getSourceNames } = require("devtools/client/shared/source-utils"); > +const { L10N } = require("../utils"); > + > +Object.defineProperty(this, "unknownSourceString", (function () { This seems like overkill for lazilyloading this one string, which is already loaded via the frame component. If we need to lazy load some L10N stuff (for the shared components? but like I said, this is already included via frame component), we should add it to utils @@ +17,5 @@ > + return { > + get() { > + if (!cached) { > + const COMPONENTS_STRINGS_URI = "chrome://devtools/locale/components.properties"; > + const componentsL10N = new ViewHelpers.L10N(COMPONENTS_STRINGS_URI); If we need to use ViewHelpers, we should explicitly require it above; this only works because we're using browser loader @@ +38,5 @@ > + for (let i = 0, length = label.length; i < length; i++) { > + const piece = label[i]; > + > + if (isSavedFrame(piece)) { > + const { short } = getSourceNames(piece.source, unknownSourceString); Or maybe we should just have the utility function getSourceName just provide it's own unknownSourceString. L10N makes modularization of this stuff hard. @@ +44,5 @@ > + } else if (piece === "noStack") { > + sanitized[i] = L10N.getStr("tree-item.nostack"); > + } else if (piece === "noFilename") { > + sanitized[i] = L10N.getStr("tree-item.nofilename"); > + } else if (piece === "JS::ubi::RootList") { preferred `const ROOT_LIST = "JS::ubi::RootList"` with the rest of the consts at the top of the file @@ +173,5 @@ > + const target = d3.select("#graph-target"); > + > + let zoom = this.state.zoom; > + if (!zoom) { > + zoom = d3.behavior.zoom().on("zoom", function() { I know I ran into issues with leaks when not unbinding this zoom in the web audio tool https://github.com/mozilla/gecko-dev/blob/78520c7e6684244e3c8a916b2181c1c2cf0a2e0a/devtools/client/webaudioeditor/views/context.js#L61-L65 edit: looks like you took care of all that here \m/ ::: devtools/client/themes/memory.css @@ +223,5 @@ > /** > * Main panel > */ > > +.vbox { Wonder if we should start adding these in a shared css file for dexulification?

Regarding where to store the splitter size, I will quote myself from the HSplitBox bug: (In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from bug 1249147 comment #9) > If the width is not a prop, then it can't be maintained across component > setup/teardown cycles. For example, if I drag to a certain width, switch > views to something that doesn't use this component anymore so it is > unmounted/destroyed, and then switch back to the original view again, I want > my width to be preserved. If we don't have the width as a prop and keep it > in state, it will not be preserved. This means that callers of the component > *must* maintain that state somewhere, explicitly pass it in as a prop, and > handle updating that state via onResize. Luckily, this is pretty > straightforward, see the WIP patch in bug 1149385. > > I went through this same thing with the tree component wrt focus and > expansion. It turns out that if you want state to persist, you have to > persist that state. Obvious, when said that way. Slightly annoying with a > little boiler plate, but very straightforward, non-magical, and explicit.

Addresses review comments.

Rebased. New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7ce84fc98d4

Ok the blocker patch had some issues and has been updated. New try push with new blocker patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74cb965ccbc1

Note that this depends on the patch in bug 1249147, which is also marked checkin-needed.