* SUMMARY JavaScript Heap Allocations Timeline. First draft. This gets us: - timeline graph (snapshots in the timeline, start/stop/periodic and manually triggered) - summary view (breakdown top objects by size / count, most are internal) - instances view (list of each object and its own size, not yet retained size) - diff view (all the added objects between the two snapshots) Next on the agenda: - a way to see the "Path to GC Root" per instance - to see the entire path to the object, like "window.foo[1].bar" or ""point.inner in closure scope of window.methods[0]" - attribute internal objects to the non-internal (retained size / dominated size) - for instance associate CodeBlocks/Executables with the Function that "owns" it - update snapshot data if objects get collected while still recording - more useful for developers to see only the live objects and not count objects that may have eventually gotten collected
Created attachment 273533 [details] [PATCH] Proposed Fix Relies on a not-yet-landed patch, so no cq.
Created attachment 273534 [details] [IMAGE] List and Overview Graph
Created attachment 273535 [details] [IMAGE] Summary View
Created attachment 273536 [details] [IMAGE] Instances View (strings) - Snapshot Diff
<rdar://problem/25078088>
Created attachment 273537 [details] [IMAGE] Instances View (objects) - Batched when >100
Created attachment 273538 [details] [IMAGE] Log Value - Context Menu - if object is still alive
Created attachment 273540 [details] [PATCH] Part 2 - Click on Timeline Graph -> Directly to ContentView
(In reply to comment #3) > Created attachment 273535 [details] > [IMAGE] Summary View Nice! Some graph coloring suggestions: 1. Class Size/Class Count graphs: I think we should be consistent with the colors used to denote different types: "string" should have the same color in both graphs. 2. Allocation Size graph: what about using a different pool of colors, to help make this graph stand out from the others? Maybe ramp two complementary colors, a cool and warm color, to show Small to Very Large.
Comment on attachment 273533 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=273533&action=review > Source/WebInspectorUI/UserInterface/Models/HeapSnapshotDiff.js:75 > + // FIXME: This only includes the newly added instances. Should we do anything with the removed instances? I do think it would be useful to track what goes away, that could help you solve a bug with weak map references or some other collected objects you didn't expect to go away. > Source/WebInspectorUI/UserInterface/Views/FormattedValue.js:160 > + span.textContent = description.substring(0, description.indexOf("(")); Trim spaces after the substring too? > Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineDataGridNode.js:36 > + name: WebInspector.UIString("Snapshot #%d").format(this._record.heapSnapshot.identifier), Drop the #. > Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineView.js:172 > + let diffComponent = new WebInspector.HierarchicalPathComponent(WebInspector.UIString("Snapshot Diff (#%d to #%d)").format(firstSnapshotIdentifier, secondSnapshotIdentifier), "snapshot-diff-icon", "snapshot-diff"); Is it really %d "to" %d? 1 to 3 would not fully take into account additions or removals in snapshot 2. Should it be "%d and %d"? I am not fond of the #s either. Al other labels say "Comparison". This is the first use of "Diff". Should be consistent. > Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineView.js:194 > + treeElementPathComponentSelected(event) > + { > + // FIXME: Nothing. The sidebar will soon be removed. > + } I think with Matt's change today this isn't needed. > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotClassDataGridNode.js:55 > + percentElement.textContent = percent.toFixed(1) + "%"; Use your new helper function. > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:66 > + percentElement.textContent = percent.toFixed(1) + "%"; Helper. > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:80 > + idElement.textContent = "@" + id + " "; I'm still meh about showing these. I don't think they provide any value to the user. Maybe only show it if we have not other data to show? > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstancesContentView.css:120 > + text-indent: 0; /* Do not inherit the DataGrid's indent. */ Hmm, annoying. > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotSummaryContentView.css:111 > + background: var(--memory-javascript-fill-color); Great! > Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js:259 > + return WebInspector.UIString("Snapshot #%d").format(timelineRecord.heapSnapshot.identifier); Drop the #.
(In reply to comment #9) > (In reply to comment #3) > > Created attachment 273535 [details] > > [IMAGE] Summary View > > Nice! Some graph coloring suggestions: > > 1. Class Size/Class Count graphs: I think we should be consistent with the > colors used to denote different types: "string" should have the same color > in both graphs. > > 2. Allocation Size graph: what about using a different pool of colors, to > help make this graph stand out from the others? Maybe ramp two complementary > colors, a cool and warm color, to show Small to Very Large. (In reply to comment #10) > Comment on attachment 273533 [details] > > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:80 > > + idElement.textContent = "@" + id + " "; > > I'm still meh about showing these. I don't think they provide any value to > the user. Maybe only show it if we have not other data to show? There is still opportunity to change the UI in many ways. I want to continue to add functionality and I'll talk with both of you about refining the UI over time. As it stands, there still isn't a way to turn on the memory instruments without manual intervention, so the UI can be considered hidden at this point.
<http://trac.webkit.org/changeset/197954>