- Revision
- 201833
- Author
- bb...@apple.com
- Date
- 2016-06-08 14:17:54 -0700 (Wed, 08 Jun 2016)
Log Message
Web Inspector: reduce redundant attribute modification updates in DOMTreeUpdater and DOMTreeElement
https://bugs.webkit.org/show_bug.cgi?id=158504
<rdar://problem/25561452>
Reviewed by Timothy Hatcher.
When the frontend gets lots of DOM.attributeModified events, it forwards these on to
DOMTreeUpdater, which pushes a record for every single modification. It then updates
the DOM elements with the attibute changes on an animation frame. However, since it
doesn't do any deduplication of the modification records, a lot of time is wasted
on updating DOMTreeElements with intermediate (non-final) attribute values.
This patch rewrites DOMTreeUpdater to precisely track which nodes and attributes
of each node need to be updated on the next animation frame. This is done using
Sets and Maps that only hold onto the most recent attribute values rather than
pushing a record object for every single mutation.
This improves the performance of the Elements tab on an SVG particle simulator
dramatically so that the Inspector will not immediately hang. It still only achieves
a few updates per second in this case, so there is still optimization to be done on
the frontend and throttling to be done on the backend.
* UserInterface/Views/DOMTreeElement.js:
(WebInspector.DOMTreeElement):
(WebInspector.DOMTreeElement.prototype.attributeDidChange):
(WebInspector.DOMTreeElement.prototype._buildAttributeDOM):
(WebInspector.DOMTreeElement.prototype._markNodeChanged):
(WebInspector.DOMTreeElement.prototype._nodeChangedAnimationEnd):
(WebInspector.DOMTreeElement.prototype._fireDidChange):
(WebInspector.DOMTreeElement.prototype.nodeStateChanged): Deleted.
Simplify the list of modified attributes a little bit. This still uses a worklist
approach, so it's possible that duplicate updates for the same attribute could accumulate
if DOMTreeUpdater pushes updates faster than DOMTreeElement can render them.
* UserInterface/Views/DOMTreeUpdater.js:
(WebInspector.DOMTreeUpdater):
(WebInspector.DOMTreeUpdater.prototype._attributesUpdated):
(WebInspector.DOMTreeUpdater.prototype._characterDataModified):
(WebInspector.DOMTreeUpdater.prototype._nodeAttributeModified):
(WebInspector.DOMTreeUpdater.prototype._nodeInserted):
(WebInspector.DOMTreeUpdater.prototype._nodeRemoved):
(WebInspector.DOMTreeUpdater.prototype._childNodeCountUpdated):
(WebInspector.DOMTreeUpdater.prototype._updateModifiedNodes):
(WebInspector.DOMTreeUpdater.prototype._reset):
Rewrite this class to separately track insertions, deletions, and modifications. Use
Sets and Maps so redundant entries are not kept around. Split the main work loop
and use fewer enum-like properties to control how each DOM element change is handled.
Attempt to update all inserted children before modifying their attributes. This
wasn't done previously, but enough duplicate attribute modifications occurred that
usually some of them would be processed after being added to the tree. There is only
one chance to do this now.
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (201832 => 201833)
--- trunk/Source/WebInspectorUI/ChangeLog 2016-06-08 21:15:09 UTC (rev 201832)
+++ trunk/Source/WebInspectorUI/ChangeLog 2016-06-08 21:17:54 UTC (rev 201833)
@@ -1,3 +1,58 @@
+2016-06-08 Brian Burg <bb...@apple.com>
+
+ Web Inspector: reduce redundant attribute modification updates in DOMTreeUpdater and DOMTreeElement
+ https://bugs.webkit.org/show_bug.cgi?id=158504
+ <rdar://problem/25561452>
+
+ Reviewed by Timothy Hatcher.
+
+ When the frontend gets lots of DOM.attributeModified events, it forwards these on to
+ DOMTreeUpdater, which pushes a record for every single modification. It then updates
+ the DOM elements with the attibute changes on an animation frame. However, since it
+ doesn't do any deduplication of the modification records, a lot of time is wasted
+ on updating DOMTreeElements with intermediate (non-final) attribute values.
+
+ This patch rewrites DOMTreeUpdater to precisely track which nodes and attributes
+ of each node need to be updated on the next animation frame. This is done using
+ Sets and Maps that only hold onto the most recent attribute values rather than
+ pushing a record object for every single mutation.
+
+ This improves the performance of the Elements tab on an SVG particle simulator
+ dramatically so that the Inspector will not immediately hang. It still only achieves
+ a few updates per second in this case, so there is still optimization to be done on
+ the frontend and throttling to be done on the backend.
+
+ * UserInterface/Views/DOMTreeElement.js:
+ (WebInspector.DOMTreeElement):
+ (WebInspector.DOMTreeElement.prototype.attributeDidChange):
+ (WebInspector.DOMTreeElement.prototype._buildAttributeDOM):
+ (WebInspector.DOMTreeElement.prototype._markNodeChanged):
+ (WebInspector.DOMTreeElement.prototype._nodeChangedAnimationEnd):
+ (WebInspector.DOMTreeElement.prototype._fireDidChange):
+ (WebInspector.DOMTreeElement.prototype.nodeStateChanged): Deleted.
+ Simplify the list of modified attributes a little bit. This still uses a worklist
+ approach, so it's possible that duplicate updates for the same attribute could accumulate
+ if DOMTreeUpdater pushes updates faster than DOMTreeElement can render them.
+
+ * UserInterface/Views/DOMTreeUpdater.js:
+ (WebInspector.DOMTreeUpdater):
+ (WebInspector.DOMTreeUpdater.prototype._attributesUpdated):
+ (WebInspector.DOMTreeUpdater.prototype._characterDataModified):
+ (WebInspector.DOMTreeUpdater.prototype._nodeAttributeModified):
+ (WebInspector.DOMTreeUpdater.prototype._nodeInserted):
+ (WebInspector.DOMTreeUpdater.prototype._nodeRemoved):
+ (WebInspector.DOMTreeUpdater.prototype._childNodeCountUpdated):
+ (WebInspector.DOMTreeUpdater.prototype._updateModifiedNodes):
+ (WebInspector.DOMTreeUpdater.prototype._reset):
+ Rewrite this class to separately track insertions, deletions, and modifications. Use
+ Sets and Maps so redundant entries are not kept around. Split the main work loop
+ and use fewer enum-like properties to control how each DOM element change is handled.
+
+ Attempt to update all inserted children before modifying their attributes. This
+ wasn't done previously, but enough duplicate attribute modifications occurred that
+ usually some of them would be processed after being added to the tree. There is only
+ one chance to do this now.
+
2016-06-08 Nikita Vasilyev <nvasil...@apple.com>
REGRESSION (r158219): Web Inspector: Border under the default Timeline content view is too thick
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js (201832 => 201833)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js 2016-06-08 21:15:09 UTC (rev 201832)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js 2016-06-08 21:17:54 UTC (rev 201833)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008, 2013, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2013, 2015, 2016 Apple Inc. All rights reserved.
* Copyright (C) 2008 Matt Lilek <web...@mattlilek.com>
* Copyright (C) 2009 Joseph Pecoraro
*
@@ -42,7 +42,7 @@
this._searchQuery = null;
this._expandedChildrenLimit = WebInspector.DOMTreeElement.InitialChildrenLimit;
- this._nodeStateChanges = [];
+ this._recentlyModifiedAttributes = [];
this._boundNodeChangedAnimationEnd = this._nodeChangedAnimationEnd.bind(this);
node.addEventListener(WebInspector.DOMNode.Event.EnabledPseudoClassesChanged, this._nodePseudoClassesDidChange, this);
@@ -197,12 +197,9 @@
return count;
}
- nodeStateChanged(change)
+ attributeDidChange(name)
{
- if (!change)
- return;
-
- this._nodeStateChanges.push(change);
+ this._recentlyModifiedAttributes.push({name});
}
showChildNode(node)
@@ -1148,9 +1145,9 @@
if (hasText)
attrSpanElement.append("\"");
- for (let change of this._nodeStateChanges) {
- if (change.type === WebInspector.DOMTreeElement.ChangeType.Attribute && change.attribute === name)
- change.element = hasText ? attrValueElement : attrNameElement;
+ for (let attribute of this._recentlyModifiedAttributes) {
+ if (attribute.name === name)
+ attribute.element = hasText ? attrValueElement : attrNameElement;
}
}
@@ -1498,8 +1495,8 @@
_markNodeChanged()
{
- for (let change of this._nodeStateChanges) {
- let element = change.element;
+ for (let attribute of this._recentlyModifiedAttributes) {
+ let element = attribute.element;
if (!element)
continue;
@@ -1515,9 +1512,9 @@
element.classList.remove("node-state-changed");
element.removeEventListener("animationend", this._boundNodeChangedAnimationEnd);
- for (let i = this._nodeStateChanges.length - 1; i >= 0; --i) {
- if (this._nodeStateChanges[i].element === element)
- this._nodeStateChanges.splice(i, 1);
+ for (let i = this._recentlyModifiedAttributes.length - 1; i >= 0; --i) {
+ if (this._recentlyModifiedAttributes[i].element === element)
+ this._recentlyModifiedAttributes.splice(i, 1);
}
}
@@ -1533,8 +1530,7 @@
{
super._fireDidChange();
- if (this._nodeStateChanges)
- this._markNodeChanged();
+ this._markNodeChanged();
}
handleEvent(event)
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js (201832 => 201833)
--- trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js 2016-06-08 21:15:09 UTC (rev 201832)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js 2016-06-08 21:17:54 UTC (rev 201833)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2013, 2016 Apple Inc. All rights reserved.
* Copyright (C) 2008 Matt Lilek <web...@mattlilek.com>
* Copyright (C) 2009 Joseph Pecoraro
*
@@ -39,7 +39,15 @@
WebInspector.domTreeManager.addEventListener(WebInspector.DOMTreeManager.Event.ChildNodeCountUpdated, this._childNodeCountUpdated, this);
this._treeOutline = treeOutline;
- this._recentlyModifiedNodes = [];
+
+ this._recentlyInsertedNodes = new Map;
+ this._recentlyDeletedNodes = new Map;
+ this._recentlyModifiedNodes = new Set;
+ // Map from attribute names to nodes that had the attributes.
+ this._recentlyModifiedAttributes = new Map;
+
+ // Dummy "attribute" that is used to track textContent changes.
+ this._textContentAttributeSymbol = Symbol("text-content-attribute");
};
WebInspector.DOMTreeUpdater.prototype = {
@@ -55,28 +63,37 @@
_attributesUpdated: function(event)
{
- this._recentlyModifiedNodes.push({node: event.data.node, updated: true, attribute: event.data.name});
- if (this._treeOutline._visible)
- this.onNextFrame._updateModifiedNodes();
+ let {node, name} = event.data;
+ this._nodeAttributeModified(node, name);
},
_characterDataModified: function(event)
{
- this._recentlyModifiedNodes.push({node: event.data.node, updated: true});
+ let {node} = event.data;
+ this._nodeAttributeModified(node, this._textContentAttributeSymbol);
+ },
+
+ _nodeAttributeModified: function(node, attribute)
+ {
+ if (!this._recentlyModifiedAttributes.has(attribute))
+ this._recentlyModifiedAttributes.set(attribute, new Set);
+ this._recentlyModifiedAttributes.get(attribute).add(node);
+ this._recentlyModifiedNodes.add(node);
+
if (this._treeOutline._visible)
this.onNextFrame._updateModifiedNodes();
- },
+ },
_nodeInserted: function(event)
{
- this._recentlyModifiedNodes.push({node: event.data.node, parent: event.data.parent, inserted: true});
+ this._recentlyInsertedNodes.set(event.data.node, {parent: event.data.parent});
if (this._treeOutline._visible)
this.onNextFrame._updateModifiedNodes();
},
_nodeRemoved: function(event)
{
- this._recentlyModifiedNodes.push({node: event.data.node, parent: event.data.parent, removed: true});
+ this._recentlyDeletedNodes.set(event.data.node, {parent: event.data.parent});
if (this._treeOutline._visible)
this.onNextFrame._updateModifiedNodes();
},
@@ -90,46 +107,53 @@
_updateModifiedNodes: function()
{
- let updatedParentTreeElements = [];
- for (let recentlyModifiedNode of this._recentlyModifiedNodes) {
- let parent = recentlyModifiedNode.parent;
- let node = recentlyModifiedNode.node;
- let changeInfo = null;
- if (recentlyModifiedNode.attribute)
- changeInfo = {type: WebInspector.DOMTreeElement.ChangeType.Attribute, attribute: recentlyModifiedNode.attribute};
+ // Update for insertions and deletions before attribute modifications. This ensures
+ // tree elements get created for newly attached children before we try to update them.
+ let parentElementsToUpdate = new Set;
+ let markNodeParentForUpdate = (value, key, map) => {
+ let parentNode = value.parent;
+ let parentTreeElement = this._treeOutline.findTreeElement(parentNode);
+ if (parentTreeElement)
+ parentElementsToUpdate.add(parentTreeElement);
+ };
+ this._recentlyInsertedNodes.forEach(markNodeParentForUpdate);
+ this._recentlyDeletedNodes.forEach(markNodeParentForUpdate);
- if (recentlyModifiedNode.updated) {
- let nodeTreeElement = this._treeOutline.findTreeElement(node);
- if (!nodeTreeElement)
+ for (let parentTreeElement of parentElementsToUpdate) {
+ parentTreeElement.updateTitle();
+ parentTreeElement.updateChildren();
+ }
+
+ for (let node of this._recentlyModifiedNodes.values()) {
+ let nodeTreeElement = this._treeOutline.findTreeElement(node);
+ if (!nodeTreeElement)
+ return;
+
+ for (let [attribute, nodes] of this._recentlyModifiedAttributes.entries()) {
+ // Don't report textContent changes as attribute modifications.
+ if (attribute === this._textContentAttributeSymbol)
continue;
- if (changeInfo)
- nodeTreeElement.nodeStateChanged(changeInfo);
-
- nodeTreeElement.updateTitle();
+ if (nodes.has(node))
+ nodeTreeElement.attributeDidChange(attribute);
}
- if (!parent)
- continue;
-
- let parentNodeItem = this._treeOutline.findTreeElement(parent);
- if (parentNodeItem && !parentNodeItem.alreadyUpdatedChildren) {
- parentNodeItem.updateTitle();
- parentNodeItem.updateChildren();
- parentNodeItem.alreadyUpdatedChildren = true;
- updatedParentTreeElements.push(parentNodeItem);
- }
+ nodeTreeElement.updateTitle();
}
- for (let i = 0; i < updatedParentTreeElements.length; ++i)
- updatedParentTreeElements[i].alreadyUpdatedChildren = null;
-
- this._recentlyModifiedNodes = [];
+ this._recentlyInsertedNodes.clear();
+ this._recentlyDeletedNodes.clear();
+ this._recentlyModifiedNodes.clear();
+ this._recentlyModifiedAttributes.clear();
},
_reset: function()
{
WebInspector.domTreeManager.hideDOMNodeHighlight();
- this._recentlyModifiedNodes = [];
+
+ this._recentlyInsertedNodes.clear();
+ this._recentlyDeletedNodes.clear();
+ this._recentlyModifiedNodes.clear();
+ this._recentlyModifiedAttributes.clear();
}
};
