We should be able to take a heap snapshot from the UI and get all of the data in bug 960765 but *also* we should be able to inspect individual object state and references to other objects as they were at the time of the snapshot.

Nick, is this ticket for the full snapshot for the entire heap or is this a snapshot in time off the live memory tool?

The full snapshot of the entire heap.

4 "preset" breakdowns for this view (may not implement them all this patch, but for reference): [11:53pm] fitzgen: coarse type => objects and strings by allocation site, others by internal type [11:53pm] fitzgen: allocation site => count [11:53pm] fitzgen: object class => count [11:54pm] fitzgen: if show-platform-data is enabled, internalType => count

First pass at the tree. Needs a lot of styling follow ups. https://treeherder.mozilla.org/#/jobs?repo=try&revision=7cd9dc4e670c

Comment on attachment 8677161 [details] [diff] [review] 960776-heap-view.patch Review of attachment 8677161 [details] [diff] [review]: ----------------------------------------------------------------- Wasn't as bad as you made it seem like it would be over IRC. The tree widget really needs tests. It is the core of this tool's UI. Especially if this is being groomed as the One True Tree Widget. Really don't want to end up in a situation where everything is really solid all the way from the platform up through the RDP and the workers and analyses except things break down in the very last step when displaying the data. Only as strong as the weakest link mumble mumble. Also, stripDynamicProps is pretty smelly to me. I think that unless the IDs are nondeterministic due to ordering of tests (which I think is not the case given the new test here) then we should just modify the EXPECTEDs to include the IDs. ::: devtools/client/memory/components/tree.js @@ +1,3 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this file, > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ This file should not be in devtools/client/memory, more like devtools/client/shared if we have such a directory. This tree view is the foundation of our tool's UI: it really deserves some unit tests. @@ +396,5 @@ > + }); > + }, > + > + /** > + * TODO FITZGEN All this stuff needs doc/usage comments. ::: devtools/shared/heapsnapshot/tests/unit/head_heapsnapshot.js @@ +161,5 @@ > + delete obj.parent; > + if (obj.children) { > + obj.children.forEach(stripDynamicProps); > + } > + return obj; Does the order that the tests run affect the id each node gets because the ids are global rather than per-census? Or are you just trying to avoid adding an id to each test's EXPECTED? If the latter, I'd prefer to add the id to each test's EXPECTED. Regardless, a good rule of thumb: either modify the object and don't return anything, or do not modify the object and return a new copy. Don't modify the in-param and return it (implies that it is a new object, when it isn't; sets people up for footgun'ing). ::: devtools/shared/heapsnapshot/tests/unit/test_census-tree-node-04.js @@ +12,5 @@ > + let func = children.find(x => x.id === 5); > + let array = children.find(x => x.id === 6); > + > + ok(id === 1, "real root has an id"); > + ok(other.parent === id, "root nodes use the real root as parent ID"); "children of root use the root's id and their parent id"

Comment on attachment 8677161 [details] [diff] [review] 960776-heap-view.patch Review of attachment 8677161 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/memory/components/tree.js @@ +1,3 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this file, > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ Until it's used by something else, I'd rather this not be in shared, as this isn't the shared tree widget yet @@ +396,5 @@ > + }); > + }, > + > + /** > + * TODO FITZGEN Do any of these TODO mean anything? ::: devtools/shared/heapsnapshot/tests/unit/head_heapsnapshot.js @@ +161,5 @@ > + delete obj.parent; > + if (obj.children) { > + obj.children.forEach(stripDynamicProps); > + } > + return obj; Yeah mostly a quick hack around. I think IDs are fresh for each test (xpcshell anyway) so the IDs are most likely deterministic surprisingly

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6) > Comment on attachment 8677161 [details] [diff] [review] > 960776-heap-view.patch > > Review of attachment 8677161 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: devtools/client/memory/components/tree.js > @@ +1,3 @@ > > +/* This Source Code Form is subject to the terms of the Mozilla Public > > + * License, v. 2.0. If a copy of the MPL was not distributed with this file, > > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > Until it's used by something else, I'd rather this not be in shared, as this > isn't the shared tree widget yet > > @@ +396,5 @@ > > + }); > > + }, > > + > > + /** > > + * TODO FITZGEN > > Do any of these TODO mean anything? They are placeholders for doc/usage comments.

Comment on attachment 8678269 [details] [diff] [review] 960776-heap-view.patch Review of attachment 8678269 [details] [diff] [review]: ----------------------------------------------------------------- I think you will need to rebase on top latest m-c. Thanks for the tests, those are really nice.

Comment on attachment 8678269 [details] [diff] [review] 960776-heap-view.patch Review of attachment 8678269 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/shared/heapsnapshot/census-tree-node.js @@ -207,5 @@ > } > > - // Loop through each frame in the stack and get or create a CensusTreeNode for > - // the frame. > - Why delete this comment? @@ +390,5 @@ > this.totalBytes = 0; > this.count = 0; > this.totalCount = 0; > this.children = undefined; > + this.id = INC++; Set `this.parent = undefined;` here too.

Backed out for XPCShell failures: https://hg.mozilla.org/integration/fx-team/rev/9e60fa0fc1e3 Failed checkin: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=335ea7a95492 19:25:09 INFO - TEST-START | devtools/shared/heapsnapshot/tests/unit/test_census-tree-node-01.js 19:25:09 WARNING - TEST-UNEXPECTED-FAIL | devtools/shared/heapsnapshot/tests/unit/test_census-tree-node-01.js | xpcshell return code: 0 19:25:09 INFO - TEST-INFO took 550ms 19:25:09 INFO - >>>>>>> 19:25:09 INFO - (xpcshell/head.js) | test MAIN run_test pending (1) 19:25:09 INFO - PROCESS | 11760 | HEAPSNAPSHOT-TEST: Generating CensusTreeNode from report: 19:25:09 INFO - PROCESS | 11760 | HEAPSNAPSHOT-TEST: breakdown: { 19:25:09 INFO - PROCESS | 11760 | "by": "internalType", 19:25:09 INFO - PROCESS | 11760 | "then": { 19:25:09 INFO - PROCESS | 11760 | "by": "count", 19:25:09 INFO - PROCESS | 11760 | "count": true, 19:25:09 INFO - PROCESS | 11760 | "bytes": true 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | HEAPSNAPSHOT-TEST: report: { 19:25:09 INFO - PROCESS | 11760 | "JSObject": { 19:25:09 INFO - PROCESS | 11760 | "bytes": 100, 19:25:09 INFO - PROCESS | 11760 | "count": 10 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | "js::Shape": { 19:25:09 INFO - PROCESS | 11760 | "bytes": 500, 19:25:09 INFO - PROCESS | 11760 | "count": 50 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | "JSString": { 19:25:09 INFO - PROCESS | 11760 | "bytes": 0, 19:25:09 INFO - PROCESS | 11760 | "count": 0 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | HEAPSNAPSHOT-TEST: expected: { 19:25:09 INFO - PROCESS | 11760 | "name": null, 19:25:09 INFO - PROCESS | 11760 | "bytes": 0, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 600, 19:25:09 INFO - PROCESS | 11760 | "count": 0, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 60, 19:25:09 INFO - PROCESS | 11760 | "children": [ 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "js::Shape", 19:25:09 INFO - PROCESS | 11760 | "bytes": 500, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 500, 19:25:09 INFO - PROCESS | 11760 | "count": 50, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 50, 19:25:09 INFO - PROCESS | 11760 | "id": 2, 19:25:09 INFO - PROCESS | 11760 | "parent": 1 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "JSObject", 19:25:09 INFO - PROCESS | 11760 | "bytes": 100, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 100, 19:25:09 INFO - PROCESS | 11760 | "count": 10, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 10, 19:25:09 INFO - PROCESS | 11760 | "id": 3, 19:25:09 INFO - PROCESS | 11760 | "parent": 1 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "JSString", 19:25:09 INFO - PROCESS | 11760 | "bytes": 0, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 0, 19:25:09 INFO - PROCESS | 11760 | "count": 0, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 0, 19:25:09 INFO - PROCESS | 11760 | "id": 4, 19:25:09 INFO - PROCESS | 11760 | "parent": 1 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | ], 19:25:09 INFO - PROCESS | 11760 | "id": 1 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | HEAPSNAPSHOT-TEST: actual: { 19:25:09 INFO - PROCESS | 11760 | "name": null, 19:25:09 INFO - PROCESS | 11760 | "bytes": 0, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 600, 19:25:09 INFO - PROCESS | 11760 | "count": 0, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 60, 19:25:09 INFO - PROCESS | 11760 | "children": [ 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "js::Shape", 19:25:09 INFO - PROCESS | 11760 | "bytes": 500, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 500, 19:25:09 INFO - PROCESS | 11760 | "count": 50, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 50, 19:25:09 INFO - PROCESS | 11760 | "id": 2, 19:25:09 INFO - PROCESS | 11760 | "parent": 0 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "JSObject", 19:25:09 INFO - PROCESS | 11760 | "bytes": 100, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 100, 19:25:09 INFO - PROCESS | 11760 | "count": 10, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 10, 19:25:09 INFO - PROCESS | 11760 | "id": 1, 19:25:09 INFO - PROCESS | 11760 | "parent": 0 19:25:09 INFO - PROCESS | 11760 | }, 19:25:09 INFO - PROCESS | 11760 | { 19:25:09 INFO - PROCESS | 11760 | "name": "JSString", 19:25:09 INFO - PROCESS | 11760 | "bytes": 0, 19:25:09 INFO - PROCESS | 11760 | "totalBytes": 0, 19:25:09 INFO - PROCESS | 11760 | "count": 0, 19:25:09 INFO - PROCESS | 11760 | "totalCount": 0, 19:25:09 INFO - PROCESS | 11760 | "id": 3, 19:25:09 INFO - PROCESS | 11760 | "parent": 0 19:25:09 INFO - PROCESS | 11760 | } 19:25:09 INFO - PROCESS | 11760 | ], 19:25:09 INFO - PROCESS | 11760 | "id": 0 19:25:09 INFO - PROCESS | 11760 | }

Looks like a bad rebased -- added parent/id props to census tree nodes after all of Nick's patches landed from end of last week, working good now