Profiles view should support drawing pie-charts based on the data received from the inspector memory agent.
Created attachment 144541 [details] Patch
Created attachment 144543 [details] Screenshot
All this functionality is still experimental.
Comment on attachment 144541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144541&action=review > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:56 > + this._pieChart.updateSize(); Making chart a view would do this for you automatically. > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:113 > + profileHeader.memoryBlock = memoryBlock; Are these public fields on the header? Do they need to be public? > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:186 > +WebInspector.MemoryBlockViewProperties = function(fillStyle, name, description) Missing constructor annotation. > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:188 > + this.fillStyle = fillStyle; Are these all public (i.e. visible outside this file?) > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:193 > +WebInspector._standardMemoryBlocks= null; Please do not define properties on the global inspector namespace. > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:195 > +WebInspector.MemoryBlockViewProperties.initialize = function() should be private > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:204 > + addBlock("rgba(240, 240, 250, 0.8)", "ProcessPrivateMemory", "Total"); These should be l18n ready for consistency. > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:209 > +WebInspector.MemoryBlockViewProperties.forMemoryBlock = function(memoryBlock) should be private > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:220 > +WebInspector.NativeMemoryPieChart = function(memorySnapshot) Missing @constructor declaration. I guess you have not tried compiling it. > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:257 > + paint: function() I think it makes sense to extract a generic PieChart component. > Source/WebCore/inspector/front-end/heapProfiler.css:264 > +.memory-pie-chart-container { Is native memory pie chart a part of the heap profiler?
(In reply to comment #4) > (From update of attachment 144541 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144541&action=review > > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:56 > > + this._pieChart.updateSize(); > > Making chart a view would do this for you automatically. > Done. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:113 > > + profileHeader.memoryBlock = memoryBlock; > > Are these public fields on the header? Do they need to be public? > Yes, they are public on the header, isTemporary is declared on the base class. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:186 > > +WebInspector.MemoryBlockViewProperties = function(fillStyle, name, description) > > Missing constructor annotation. > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:188 > > + this.fillStyle = fillStyle; > > Are these all public (i.e. visible outside this file?) > They are public in this file. And I usually mark such fields as regular public fields(no _ prefix). This way you can easily see public API of any class. Another arguable point in marking file-private fields with underscore is how overridable methods should be named. Do they need to have _ prefix if they are only overriden in classes in the same file? Anyways, changed all fields to start with _. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:193 > > +WebInspector._standardMemoryBlocks= null; > > Please do not define properties on the global inspector namespace. > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:195 > > +WebInspector.MemoryBlockViewProperties.initialize = function() > > should be private > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:204 > > + addBlock("rgba(240, 240, 250, 0.8)", "ProcessPrivateMemory", "Total"); > > These should be l18n ready for consistency. > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:209 > > +WebInspector.MemoryBlockViewProperties.forMemoryBlock = function(memoryBlock) > > should be private > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:220 > > +WebInspector.NativeMemoryPieChart = function(memorySnapshot) > > Missing @constructor declaration. I guess you have not tried compiling it. > Fixed. > > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:257 > > + paint: function() > > I think it makes sense to extract a generic PieChart component. > It is too early at this stage as it is not clear what the API should look like. We'll be able to extract generic functionality later. > > Source/WebCore/inspector/front-end/heapProfiler.css:264 > > +.memory-pie-chart-container { > > Is native memory pie chart a part of the heap profiler? It is hard to say. I will move this into nativeMemoryProfiler.css
Created attachment 144758 [details] Patch
Seems to me you have to have the same sets in legend and in pie chart. Looks like the legend has unnecessary item Used Heap size or you has to split JS Heap size to two (used and free) and change the legend.
Created attachment 145082 [details] Patch
(In reply to comment #7) > Seems to me you have to have the same sets in legend and in pie chart. > Looks like the legend has unnecessary item Used Heap size or you has to split JS Heap size to two (used and free) and change the legend. Done. Removed used JS heap size label.
Comment on attachment 145082 [details] Patch please remove Source/WebCore/inspector/front-end/.NativeMemorySnapshotView.js.swo from the patch
Comment on attachment 145082 [details] Patch lgtm
Created attachment 145084 [details] Patch
Comment on attachment 145084 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145084&action=review lgtm > Source/WebCore/ChangeLog:12 > + * inspector/front-end/.NativeMemorySnapshotView.js.swo: Added. nit: remove this entry
Created attachment 145086 [details] Patch
(In reply to comment #13) > (From update of attachment 145084 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145084&action=review > > lgtm > > > Source/WebCore/ChangeLog:12 > > + * inspector/front-end/.NativeMemorySnapshotView.js.swo: Added. > > nit: remove this entry Done.
Committed r119197: <http://trac.webkit.org/changeset/119197>