[webkit-changes] [198620] trunk/Source/WebInspectorUI

Thu, 24 Mar 2016 01:10:41 -0700

Title: [198620] trunk/Source/WebInspectorUI
Revision
198620
Author
[email protected]
Date
2016-03-24 01:10:01 -0700 (Thu, 24 Mar 2016)

Log Message

Web Inspector: Miscellaneous performance fixes in Timeline recording
https://bugs.webkit.org/show_bug.cgi?id=155832

Patch by Joseph Pecoraro <[email protected]> on 2016-03-24
Reviewed by Timothy Hatcher.

* UserInterface/Models/CallingContextTree.js:
(WebInspector.CCTNode.prototype.hasChildren):
No need to allocate an array with all of the properties, we can just
check if there is at least one property using a short circuit for..in.
Performance was always faster for empty, small, and large objects
in micro benchmarks.

* UserInterface/Views/DataGrid.js:
(WebInspector.DataGrid.prototype.layout):
Avoid causing DOM layout when positioning resizers. They only need
a layout if we are resizing the DataGrid, or the initial layout.

* UserInterface/Views/NavigationBar.js:
(WebInspector.NavigationBar):
(WebInspector.NavigationBar.prototype.needsLayout):
(WebInspector.NavigationBar.prototype.layout):
Avoid causing DOM layout every View layout. In fact, only do a
DOM layout when someone has triggered a needsLayout on this
navigation bar. A basic dirty layout (triggered by a parent)
should not have caused us to resize.

* UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js:
* UserInterface/Views/LayoutTimelineOverviewGraph.js:
(WebInspector.LayoutTimelineOverviewGraph.prototype.layout):
* UserInterface/Views/MemoryTimelineOverviewGraph.js:
(WebInspector.MemoryTimelineOverviewGraph.prototype.reset):
(WebInspector.MemoryTimelineOverviewGraph.prototype._updateLegend):
* UserInterface/Views/NetworkTimelineOverviewGraph.js:
* UserInterface/Views/RenderingFrameTimelineOverviewGraph.js:
(WebInspector.RenderingFrameTimelineOverviewGraph.prototype.layout):
* UserInterface/Views/ScriptTimelineOverviewGraph.js:
Avoid doing any work in non-visible graphs. This was very common
because the RenderingFrameTimelineOverviewGraph is never visible
when the other timeline graphs are, but was performing lots of work.

* UserInterface/Views/MemoryCategoryView.js:
(WebInspector.MemoryCategoryView.prototype.clear):
(WebInspector.MemoryCategoryView.prototype._updateDetails):
(WebInspector.MemoryCategoryView):
* UserInterface/Views/MemoryTimelineView.js:
(WebInspector.MemoryTimelineView.prototype.reset):
(WebInspector.MemoryTimelineView.prototype._updateUsageLegend):
(WebInspector.MemoryTimelineView.prototype._updateMaxComparisonLegend):
Cache values to avoid textContent calls even if the content did not change.
This reduces unnecessary work when the values wouldn't change.

* UserInterface/Views/TimelineRecordBar.js:
(WebInspector.TimelineRecordBar.createCombinedBars):
(WebInspector.TimelineRecordBar.prototype.set records):
Revert to fast loop and as this code path is very hot and for..of iteration
was showing up in profiles. Remove assert which seems rather pointless but
showed up in profiles.

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (198619 => 198620)


--- trunk/Source/WebInspectorUI/ChangeLog	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/ChangeLog	2016-03-24 08:10:01 UTC (rev 198620)
@@ -1,3 +1,63 @@
+2016-03-24  Joseph Pecoraro  <[email protected]>
+
+        Web Inspector: Miscellaneous performance fixes in Timeline recording
+        https://bugs.webkit.org/show_bug.cgi?id=155832
+
+        Reviewed by Timothy Hatcher.
+
+        * UserInterface/Models/CallingContextTree.js:
+        (WebInspector.CCTNode.prototype.hasChildren):
+        No need to allocate an array with all of the properties, we can just
+        check if there is at least one property using a short circuit for..in.
+        Performance was always faster for empty, small, and large objects
+        in micro benchmarks.
+
+        * UserInterface/Views/DataGrid.js:
+        (WebInspector.DataGrid.prototype.layout):
+        Avoid causing DOM layout when positioning resizers. They only need
+        a layout if we are resizing the DataGrid, or the initial layout.
+
+        * UserInterface/Views/NavigationBar.js:
+        (WebInspector.NavigationBar):
+        (WebInspector.NavigationBar.prototype.needsLayout):
+        (WebInspector.NavigationBar.prototype.layout):
+        Avoid causing DOM layout every View layout. In fact, only do a
+        DOM layout when someone has triggered a needsLayout on this
+        navigation bar. A basic dirty layout (triggered by a parent)
+        should not have caused us to resize.
+
+        * UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js:
+        * UserInterface/Views/LayoutTimelineOverviewGraph.js:
+        (WebInspector.LayoutTimelineOverviewGraph.prototype.layout):
+        * UserInterface/Views/MemoryTimelineOverviewGraph.js:
+        (WebInspector.MemoryTimelineOverviewGraph.prototype.reset):
+        (WebInspector.MemoryTimelineOverviewGraph.prototype._updateLegend):
+        * UserInterface/Views/NetworkTimelineOverviewGraph.js:
+        * UserInterface/Views/RenderingFrameTimelineOverviewGraph.js:
+        (WebInspector.RenderingFrameTimelineOverviewGraph.prototype.layout):
+        * UserInterface/Views/ScriptTimelineOverviewGraph.js:
+        Avoid doing any work in non-visible graphs. This was very common
+        because the RenderingFrameTimelineOverviewGraph is never visible
+        when the other timeline graphs are, but was performing lots of work.
+
+        * UserInterface/Views/MemoryCategoryView.js:
+        (WebInspector.MemoryCategoryView.prototype.clear):
+        (WebInspector.MemoryCategoryView.prototype._updateDetails):
+        (WebInspector.MemoryCategoryView):
+        * UserInterface/Views/MemoryTimelineView.js:
+        (WebInspector.MemoryTimelineView.prototype.reset):
+        (WebInspector.MemoryTimelineView.prototype._updateUsageLegend):
+        (WebInspector.MemoryTimelineView.prototype._updateMaxComparisonLegend):
+        Cache values to avoid textContent calls even if the content did not change.
+        This reduces unnecessary work when the values wouldn't change.
+
+        * UserInterface/Views/TimelineRecordBar.js:
+        (WebInspector.TimelineRecordBar.createCombinedBars):
+        (WebInspector.TimelineRecordBar.prototype.set records):
+        Revert to fast loop and as this code path is very hot and for..of iteration
+        was showing up in profiles. Remove assert which seems rather pointless but
+        showed up in profiles.
+
 2016-03-24  Nikita Vasilyev  <[email protected]>
 
         Web Inspector: Large repaints while typing in the console tab

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CallingContextTree.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Models/CallingContextTree.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CallingContextTree.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -244,7 +244,7 @@
 
     hasChildren()
     {
-        return !!Object.getOwnPropertyNames(this._children).length;
+        return !isEmptyObject(this._children);
     }
 
     findOrMakeChild(stackFrame)

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DataGrid.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -593,8 +593,10 @@
     //
     // If this function is not called after the DataGrid is attached to its
     // parent element, then the DataGrid's columns will not be resizable.
-    layout()
+    layout(layoutReason)
     {
+        let firstUpdate = false;
+
         // Do not attempt to use offsets if we're not attached to the document tree yet.
         if (!this._columnWidthsInitialized && this.element.offsetWidth) {
             // Give all the columns initial widths now so that during a resize,
@@ -618,10 +620,13 @@
             }
 
             this._columnWidthsInitialized = true;
+            firstUpdate = true;
         }
 
-        this._positionResizerElements();
-        this._positionHeaderViews();
+        if (layoutReason == WebInspector.View.LayoutReason.Resize || firstUpdate) {
+            this._positionResizerElements();
+            this._positionHeaderViews();
+        }
     }
 
     columnWidthsMap()

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -48,6 +48,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         this.element.removeChildren();
 
         // This may display records past the current time marker.

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/LayoutTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/LayoutTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/LayoutTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -61,6 +61,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         this._updateRowLayout(this._timelinePaintRecordRow);
         this._updateRowLayout(this._timelineLayoutRecordRow);
     }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/MemoryCategoryView.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/MemoryCategoryView.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/MemoryCategoryView.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -64,6 +64,9 @@
 
     clear()
     {
+        this._cachedMinSize = undefined;
+        this._cachedMaxSize = undefined;
+
         this._chart.clear();
         this._chart.needsLayout();
     }
@@ -109,6 +112,12 @@
 
     _updateDetails(minSize, maxSize)
     {
+        if (this._cachedMinSize === minSize && this._cachedMaxSize === maxSize)
+            return;
+
+        this._cachedMinSize = minSize;
+        this._cachedMaxSize = maxSize;
+
         this._detailsMaxElement.textContent = WebInspector.UIString("Highest: %s").format(Number.isFinite(maxSize) ? Number.bytesToString(maxSize) : emDash);
         this._detailsMinElement.textContent = WebInspector.UIString("Lowest: %s").format(Number.isFinite(minSize) ? Number.bytesToString(minSize) : emDash);
     }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -65,6 +65,7 @@
         super.reset();
 
         this._maxSize = 0;
+        this._cachedMaxSize = undefined;
 
         this._updateLegend();
         this._chart.clear();
@@ -76,6 +77,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         this._updateLegend();
         this._chart.clear();
 
@@ -165,6 +169,11 @@
 
     _updateLegend()
     {
+        if (this._cachedMaxSize === this._maxSize)
+            return;
+
+        this._cachedMaxSize = this._maxSize;
+
         if (!this._maxSize) {
             this._legendElement.hidden = true;
             this._legendElement.textContent = "";

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -135,6 +135,10 @@
 
         this._maxSize = 0;
 
+        this._cachedLegendRecord = null;
+        this._cachedLegendMaxSize = undefined;
+        this._cachedLegendCurrentSize = undefined;
+
         this._usageCircleChart.clear();
         this._usageCircleChart.needsLayout();
         this._clearUsageLegend();
@@ -244,6 +248,11 @@
 
     _updateUsageLegend(record)
     {
+        if (this._cachedLegendRecord === record)
+            return;
+
+        this._cachedLegendRecord = record;
+
         for (let {type, size} of record.categories) {
             let sizeElement = this._usageLegendSizeElementMap.get(type);
             sizeElement.textContent = Number.isFinite(size) ? Number.bytesToString(size) : emDash;
@@ -276,6 +285,12 @@
 
     _updateMaxComparisonLegend(currentSize)
     {
+        if (this._cachedLegendMaxSize === this._maxSize && this._cachedLegendCurrentSize === currentSize)
+            return;
+
+        this._cachedLegendMaxSize = this._maxSize;
+        this._cachedLegendCurrentSize = currentSize;
+
         this._maxComparisonMaximumSizeElement.textContent = Number.isFinite(this._maxSize) ? Number.bytesToString(this._maxSize) : emDash;
         this._maxComparisonCurrentSizeElement.textContent = Number.isFinite(currentSize) ? Number.bytesToString(currentSize) : emDash;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -42,6 +42,7 @@
         this.element.addEventListener("keydown", this._keyDown.bind(this), false);
         this.element.addEventListener("mousedown", this._mouseDown.bind(this), false);
 
+        this._forceLayout = false;
         this._minimumWidth = NaN;
         this._navigationItems = [];
 
@@ -160,8 +161,20 @@
         return false;
     }
 
-    layout()
+    needsLayout()
     {
+        this._forceLayout = true;
+
+        super.needsLayout();
+    }
+
+    layout(layoutReason)
+    {
+        if (layoutReason !== WebInspector.View.LayoutReason.Resize && !this._forceLayout)
+            return;
+
+        this._forceLayout = false;
+
         // Remove the collapsed style class to test if the items can fit at full width.
         this.element.classList.remove(WebInspector.NavigationBar.CollapsedStyleClassName);

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NetworkTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NetworkTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NetworkTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -65,6 +65,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         let secondsPerPixel = this.timelineOverview.secondsPerPixel;
         let recordBarIndex = 0;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -102,6 +102,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         if (!this._renderingFrameTimeline.records.length)
             return;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ScriptTimelineOverviewGraph.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ScriptTimelineOverviewGraph.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ScriptTimelineOverviewGraph.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -52,6 +52,9 @@
 
     layout()
     {
+        if (!this.visible)
+            return;
+
         let secondsPerPixel = this.timelineOverview.secondsPerPixel;
         let recordBarIndex = 0;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js (198619 => 198620)


--- trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js	2016-03-24 07:13:05 UTC (rev 198619)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js	2016-03-24 08:10:01 UTC (rev 198620)
@@ -51,7 +51,8 @@
 
         // FIXME: Do a binary search for records that fall inside start and current time.
 
-        for (var record of records) {
+        for (var i = 0; i < records.length; ++i) {
+            var record = records[i];
             if (isNaN(record.startTime))
                 continue;
 
@@ -99,7 +100,9 @@
             var inactiveEndTime = NaN;
             var inactiveRecords = [];
 
-            for (var record of visibleRecords) {
+            for (var i = 0; i < visibleRecords.length; ++i) {
+                var record = visibleRecords[i];
+
                 // Check if the previous record is far enough away to create the inactive bar.
                 if (!isNaN(inactiveStartTime) && inactiveStartTime + Math.max(inactiveEndTime - inactiveStartTime, minimumDuration) + minimumMargin <= record.startTime) {
                     createBarCallback(inactiveRecords, WebInspector.TimelineRecordBar.RenderMode.InactiveOnly);
@@ -131,7 +134,8 @@
 
         var startTimeProperty = usesActiveStartTime ? "activeStartTime" : "startTime";
 
-        for (var record of visibleRecords) {
+        for (var i = 0; i < visibleRecords.length; ++i) {
+            var record = visibleRecords[i];
             var startTime = record[startTimeProperty];
 
             // Check if the previous record is far enough away to create the active bar. We also create it now if the current record has no active state time.
@@ -194,8 +198,6 @@
 
         records = records || [];
 
-        console.assert(records instanceof Array, "records should be an array");
-
         this._records = records;
 
         // Assume all records are the same type.

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to