⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 201833 in webkit


Ignore:
Timestamp:
Jun 8, 2016, 2:17:54 PM (10 years ago)
Author:
BJ Burg
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.

Location:
trunk/Source/WebInspectorUI
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebInspectorUI/ChangeLog

    r201829 r201833  
     12016-06-08  Brian Burg  <bburg@apple.com>
     2
     3        Web Inspector: reduce redundant attribute modification updates in DOMTreeUpdater and DOMTreeElement
     4        https://bugs.webkit.org/show_bug.cgi?id=158504
     5        <rdar://problem/25561452>
     6
     7        Reviewed by Timothy Hatcher.
     8
     9        When the frontend gets lots of DOM.attributeModified events, it forwards these on to
     10        DOMTreeUpdater, which pushes a record for every single modification. It then updates
     11        the DOM elements with the attibute changes on an animation frame. However, since it
     12        doesn't do any deduplication of the modification records, a lot of time is wasted
     13        on updating DOMTreeElements with intermediate (non-final) attribute values.
     14
     15        This patch rewrites DOMTreeUpdater to precisely track which nodes and attributes
     16        of each node need to be updated on the next animation frame. This is done using
     17        Sets and Maps that only hold onto the most recent attribute values rather than
     18        pushing a record object for every single mutation.
     19
     20        This improves the performance of the Elements tab on an SVG particle simulator
     21        dramatically so that the Inspector will not immediately hang. It still only achieves
     22        a few updates per second in this case, so there is still optimization to be done on
     23        the frontend and throttling to be done on the backend.
     24
     25        * UserInterface/Views/DOMTreeElement.js:
     26        (WebInspector.DOMTreeElement):
     27        (WebInspector.DOMTreeElement.prototype.attributeDidChange):
     28        (WebInspector.DOMTreeElement.prototype._buildAttributeDOM):
     29        (WebInspector.DOMTreeElement.prototype._markNodeChanged):
     30        (WebInspector.DOMTreeElement.prototype._nodeChangedAnimationEnd):
     31        (WebInspector.DOMTreeElement.prototype._fireDidChange):
     32        (WebInspector.DOMTreeElement.prototype.nodeStateChanged): Deleted.
     33        Simplify the list of modified attributes a little bit. This still uses a worklist
     34        approach, so it's possible that duplicate updates for the same attribute could accumulate
     35        if DOMTreeUpdater pushes updates faster than DOMTreeElement can render them.
     36
     37        * UserInterface/Views/DOMTreeUpdater.js:
     38        (WebInspector.DOMTreeUpdater):
     39        (WebInspector.DOMTreeUpdater.prototype._attributesUpdated):
     40        (WebInspector.DOMTreeUpdater.prototype._characterDataModified):
     41        (WebInspector.DOMTreeUpdater.prototype._nodeAttributeModified):
     42        (WebInspector.DOMTreeUpdater.prototype._nodeInserted):
     43        (WebInspector.DOMTreeUpdater.prototype._nodeRemoved):
     44        (WebInspector.DOMTreeUpdater.prototype._childNodeCountUpdated):
     45        (WebInspector.DOMTreeUpdater.prototype._updateModifiedNodes):
     46        (WebInspector.DOMTreeUpdater.prototype._reset):
     47        Rewrite this class to separately track insertions, deletions, and modifications. Use
     48        Sets and Maps so redundant entries are not kept around. Split the main work loop
     49        and use fewer enum-like properties to control how each DOM element change is handled.
     50
     51        Attempt to update all inserted children before modifying their attributes. This
     52        wasn't done previously, but enough duplicate attribute modifications occurred that
     53        usually some of them would be processed after being added to the tree. There is only
     54        one chance to do this now.
     55
    1562016-06-08  Nikita Vasilyev  <nvasilyev@apple.com>
    257

  • trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js

    r200808 r201833  
    11/*
    2  * Copyright (C) 2007, 2008, 2013, 2015 Apple Inc.  All rights reserved.
     2 * Copyright (C) 2007, 2008, 2013, 2015, 2016 Apple Inc.  All rights reserved.
    33 * Copyright (C) 2008 Matt Lilek <webkit@mattlilek.com>
    44 * Copyright (C) 2009 Joseph Pecoraro
    … …  
    4343        this._expandedChildrenLimit = WebInspector.DOMTreeElement.InitialChildrenLimit;
    4444
    45         this._nodeStateChanges = [];
     45        this._recentlyModifiedAttributes = [];
    4646        this._boundNodeChangedAnimationEnd = this._nodeChangedAnimationEnd.bind(this);
    4747
    … …  
    198198    }
    199199
    200     nodeStateChanged(change)
    201     {
    202         if (!change)
    203             return;
    204 
    205         this._nodeStateChanges.push(change);
     200    attributeDidChange(name)
     201    {
     202        this._recentlyModifiedAttributes.push({name});
    206203    }
    207204
    … …  
    11491146            attrSpanElement.append("\"");
    11501147
    1151         for (let change of this._nodeStateChanges) {
    1152             if (change.type === WebInspector.DOMTreeElement.ChangeType.Attribute && change.attribute === name)
    1153                 change.element = hasText ? attrValueElement : attrNameElement;
     1148        for (let attribute of this._recentlyModifiedAttributes) {
     1149            if (attribute.name === name)
     1150                attribute.element = hasText ? attrValueElement : attrNameElement;
    11541151        }
    11551152    }
    … …  
    14991496    _markNodeChanged()
    15001497    {
    1501         for (let change of this._nodeStateChanges) {
    1502             let element = change.element;
     1498        for (let attribute of this._recentlyModifiedAttributes) {
     1499            let element = attribute.element;
    15031500            if (!element)
    15041501                continue;
    … …  
    15161513        element.removeEventListener("animationend", this._boundNodeChangedAnimationEnd);
    15171514
    1518         for (let i = this._nodeStateChanges.length - 1; i >= 0; --i) {
    1519             if (this._nodeStateChanges[i].element === element)
    1520                 this._nodeStateChanges.splice(i, 1);
     1515        for (let i = this._recentlyModifiedAttributes.length - 1; i >= 0; --i) {
     1516            if (this._recentlyModifiedAttributes[i].element === element)
     1517                this._recentlyModifiedAttributes.splice(i, 1);
    15211518        }
    15221519    }
    … …  
    15341531        super._fireDidChange();
    15351532
    1536         if (this._nodeStateChanges)
    1537             this._markNodeChanged();
     1533        this._markNodeChanged();
    15381534    }
    15391535

  • trunk/Source/WebInspectorUI/UserInterface/Views/DOMTreeUpdater.js

    r201778 r201833  
    11/*
    2  * Copyright (C) 2007, 2008, 2013 Apple Inc.  All rights reserved.
     2 * Copyright (C) 2007, 2008, 2013, 2016 Apple Inc.  All rights reserved.
    33 * Copyright (C) 2008 Matt Lilek <webkit@mattlilek.com>
    44 * Copyright (C) 2009 Joseph Pecoraro
    … …  
    4040
    4141    this._treeOutline = treeOutline;
    42     this._recentlyModifiedNodes = [];
     42
     43    this._recentlyInsertedNodes = new Map;
     44    this._recentlyDeletedNodes = new Map;
     45    this._recentlyModifiedNodes = new Set;
     46    // Map from attribute names to nodes that had the attributes.
     47    this._recentlyModifiedAttributes = new Map;
     48
     49    // Dummy "attribute" that is used to track textContent changes.
     50    this._textContentAttributeSymbol = Symbol("text-content-attribute");
    4351};
    4452
    … …  
    5664    _attributesUpdated: function(event)
    5765    {
    58         this._recentlyModifiedNodes.push({node: event.data.node, updated: true, attribute: event.data.name});
    59         if (this._treeOutline._visible)
    60             this.onNextFrame._updateModifiedNodes();
     66        let {node, name} = event.data;
     67        this._nodeAttributeModified(node, name);
    6168    },
    6269
    6370    _characterDataModified: function(event)
    6471    {
    65         this._recentlyModifiedNodes.push({node: event.data.node, updated: true});
     72        let {node} = event.data;
     73        this._nodeAttributeModified(node, this._textContentAttributeSymbol);
     74    },
     75
     76    _nodeAttributeModified: function(node, attribute)
     77    {
     78        if (!this._recentlyModifiedAttributes.has(attribute))
     79            this._recentlyModifiedAttributes.set(attribute, new Set);
     80        this._recentlyModifiedAttributes.get(attribute).add(node);
     81        this._recentlyModifiedNodes.add(node);
     82
    6683        if (this._treeOutline._visible)
    6784            this.onNextFrame._updateModifiedNodes();
    68     },
     85      },
    6986
    7087    _nodeInserted: function(event)
    7188    {
    72         this._recentlyModifiedNodes.push({node: event.data.node, parent: event.data.parent, inserted: true});
     89        this._recentlyInsertedNodes.set(event.data.node, {parent: event.data.parent});
    7390        if (this._treeOutline._visible)
    7491            this.onNextFrame._updateModifiedNodes();
    … …  
    7794    _nodeRemoved: function(event)
    7895    {
    79         this._recentlyModifiedNodes.push({node: event.data.node, parent: event.data.parent, removed: true});
     96        this._recentlyDeletedNodes.set(event.data.node, {parent: event.data.parent});
    8097        if (this._treeOutline._visible)
    8198            this.onNextFrame._updateModifiedNodes();
    … …  
    91108    _updateModifiedNodes: function()
    92109    {
    93         let updatedParentTreeElements = [];
    94         for (let recentlyModifiedNode of this._recentlyModifiedNodes) {
    95             let parent = recentlyModifiedNode.parent;
    96             let node = recentlyModifiedNode.node;
    97             let changeInfo = null;
    98             if (recentlyModifiedNode.attribute)
    99                 changeInfo = {type: WebInspector.DOMTreeElement.ChangeType.Attribute, attribute: recentlyModifiedNode.attribute};
     110        // Update for insertions and deletions before attribute modifications. This ensures
     111        // tree elements get created for newly attached children before we try to update them.
     112        let parentElementsToUpdate = new Set;
     113        let markNodeParentForUpdate = (value, key, map) => {
     114            let parentNode = value.parent;
     115            let parentTreeElement = this._treeOutline.findTreeElement(parentNode);
     116            if (parentTreeElement)
     117                parentElementsToUpdate.add(parentTreeElement);
     118        };
     119        this._recentlyInsertedNodes.forEach(markNodeParentForUpdate);
     120        this._recentlyDeletedNodes.forEach(markNodeParentForUpdate);
    100121
    101             if (recentlyModifiedNode.updated) {
    102                 let nodeTreeElement = this._treeOutline.findTreeElement(node);
    103                 if (!nodeTreeElement)
     122        for (let parentTreeElement of parentElementsToUpdate) {
     123            parentTreeElement.updateTitle();
     124            parentTreeElement.updateChildren();
     125        }
     126
     127        for (let node of this._recentlyModifiedNodes.values()) {
     128            let nodeTreeElement = this._treeOutline.findTreeElement(node);
     129            if (!nodeTreeElement)
     130                return;
     131
     132            for (let [attribute, nodes] of this._recentlyModifiedAttributes.entries()) {
     133                // Don't report textContent changes as attribute modifications.
     134                if (attribute === this._textContentAttributeSymbol)
    104135                    continue;
    105136
    106                 if (changeInfo)
    107                     nodeTreeElement.nodeStateChanged(changeInfo);
    108 
    109                 nodeTreeElement.updateTitle();
     137                if (nodes.has(node))
     138                    nodeTreeElement.attributeDidChange(attribute);
    110139            }
    111140
    112             if (!parent)
    113                 continue;
    114 
    115             let parentNodeItem = this._treeOutline.findTreeElement(parent);
    116             if (parentNodeItem && !parentNodeItem.alreadyUpdatedChildren) {
    117                 parentNodeItem.updateTitle();
    118                 parentNodeItem.updateChildren();
    119                 parentNodeItem.alreadyUpdatedChildren = true;
    120                 updatedParentTreeElements.push(parentNodeItem);
    121             }
     141            nodeTreeElement.updateTitle();
    122142        }
    123143
    124         for (let i = 0; i < updatedParentTreeElements.length; ++i)
    125             updatedParentTreeElements[i].alreadyUpdatedChildren = null;
    126 
    127         this._recentlyModifiedNodes = [];
     144        this._recentlyInsertedNodes.clear();
     145        this._recentlyDeletedNodes.clear();
     146        this._recentlyModifiedNodes.clear();
     147        this._recentlyModifiedAttributes.clear();
    128148    },
    129149
    … …  
    131151    {
    132152        WebInspector.domTreeManager.hideDOMNodeHighlight();
    133         this._recentlyModifiedNodes = [];
     153
     154        this._recentlyInsertedNodes.clear();
     155        this._recentlyDeletedNodes.clear();
     156        this._recentlyModifiedNodes.clear();
     157        this._recentlyModifiedAttributes.clear();
    134158    }
    135159};
Note: See TracChangeset for help on using the changeset viewer.