Commit 81d17784 authored by shinyak@chromium.org's avatar shinyak@chromium.org

Changing id, className, or attribute should invalidate distribution

https://bugs.webkit.org/show_bug.cgi?id=100738

Reviewed by Dimitri Glazkov.

PerformanceTests:

Added test code to modify id/class/attribute.

* DOM/ModifyAttribute.html: Added.
* DOM/resources/dom-perf/modify-attribute.js: Added.
(ModifyAttribute.CreateElementToSetUp):
(ModifyAttribute.ModifyId):
(ModifyAttribute.ModifyClass):
(ModifyAttribute.ModifyTitle):

Source/WebCore:

When id, className, or attribute is changed, we might have to invalidate distribution.
However, we don't want to do useless invalidation. So we consult with the RuleFeatureSet of ElementShadow
to invalidate distribution only if necessary.

For the code that className is changed, we can share a lot of code between invalidating distribution and
invalidating style. So we made checkNeedsStyleInvalidationForClassChange a template method, and share it.

Since attributeChanged() is a hot method, we don't want to make it slow. So we made one function to determine
whether we have to invalidate distribution, and make it called only if necessary. Also, we've optimized
shadowOfParentForDistribution() by making isInsertionPoint() de-virtualed. We consuded NodeFlags (IsInsertionPointFlag)
for this purpose.

We've measured how this patch makes changing attribute slow. I've measured each code 3 times.
DOM/ModifyAttribute.html is a micro benchmark which changes attribute a lot of times. The result of this benchmark
will be the most affected by this patch. However, it's only 2% performance regression.

DOM/ModifyAttribute.html
Before this patch:
        median  stdev    min    max    [ms]
  1st    494.0   3.36  490.0  502.0
  2nd    503.5   3.44  497.0  512.0
  3rd    494.0   3.48  488.0  499.0

After this patch:
        median  stdev  min      max    [ms]
  1st    504.0   2.00  501.0  509.0
  2nd    505.5   3.08  500.0  513.0
  3rd    507.0   2.32  502.0  510.0

Tests: fast/dom/shadow/distribution-attribute-modified.html
       fast/dom/shadow/distribution-className-modified.html
       fast/dom/shadow/distribution-id-modified.html
       fast/dom/shadow/reprojection-attribute-modified.html
       fast/dom/shadow/reprojection-className-modified.html
       fast/dom/shadow/reprojection-id-modified.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged):
(WebCore::HasSelectorForClassStyleFunctor::HasSelectorForClassStyleFunctor):
(HasSelectorForClassStyleFunctor):
(WebCore::HasSelectorForClassStyleFunctor::operator()): Returns true if StyleResolver::hasSelectorForClass returns true.
(WebCore):
(WebCore::HasSelectorForClassDistributionFunctor::HasSelectorForClassDistributionFunctor):
(HasSelectorForClassDistributionFunctor):
(WebCore::HasSelectorForClassDistributionFunctor::operator()): Returns true if ElementShadow::hasSelectForClass returns true.
(WebCore::checkFunctorForClassChange):
(WebCore::checkNeedsStyleInvalidationForClassChange):
(WebCore::checkNeedsDistributionInvalidationForClassChange): Extracted the implementation to checkFunctorForClassChange.
(WebCore::Element::shouldInvalidateDistributionWhenAttributeChanged):
* dom/Element.h:
(Element):
* dom/Node.h:
(WebCore::Node::isInsertionPoint):
* html/HTMLElement.h:
(HTMLElement):
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::InsertionPoint):
* html/shadow/InsertionPoint.h:
(InsertionPoint):
(WebCore::isInsertionPoint):
(WebCore::shadowOfParentForDistribution):
(WebCore::resolveReprojection):

LayoutTests:

We have test cases that id/class/attribute is changed, and thier reprojection cases.

* fast/dom/shadow/distribution-attribute-modified-expected.html: Added.
* fast/dom/shadow/distribution-attribute-modified.html: Added.
* fast/dom/shadow/distribution-className-modified-expected.html: Added.
* fast/dom/shadow/distribution-className-modified.html: Added.
* fast/dom/shadow/distribution-id-modified-expected.html: Added.
* fast/dom/shadow/distribution-id-modified.html: Added.
* fast/dom/shadow/reprojection-attribute-modified-expected.html: Added.
* fast/dom/shadow/reprojection-attribute-modified.html: Added.
* fast/dom/shadow/reprojection-className-modified-expected.html: Added.
* fast/dom/shadow/reprojection-className-modified.html: Added.
* fast/dom/shadow/reprojection-id-modified-expected.html: Added.
* fast/dom/shadow/reprojection-id-modified.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@135174 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ec65998b
2012-11-19 Shinya Kawanaka <shinyak@chromium.org>
Changing id, className, or attribute should invalidate distribution
https://bugs.webkit.org/show_bug.cgi?id=100738
Reviewed by Dimitri Glazkov.
We have test cases that id/class/attribute is changed, and thier reprojection cases.
* fast/dom/shadow/distribution-attribute-modified-expected.html: Added.
* fast/dom/shadow/distribution-attribute-modified.html: Added.
* fast/dom/shadow/distribution-className-modified-expected.html: Added.
* fast/dom/shadow/distribution-className-modified.html: Added.
* fast/dom/shadow/distribution-id-modified-expected.html: Added.
* fast/dom/shadow/distribution-id-modified.html: Added.
* fast/dom/shadow/reprojection-attribute-modified-expected.html: Added.
* fast/dom/shadow/reprojection-attribute-modified.html: Added.
* fast/dom/shadow/reprojection-className-modified-expected.html: Added.
* fast/dom/shadow/reprojection-className-modified.html: Added.
* fast/dom/shadow/reprojection-id-modified-expected.html: Added.
* fast/dom/shadow/reprojection-id-modified.html: Added.
2012-11-19 Nate Chapin <japhet@chromium.org>
Move empty loading to DocumentLoader, simplify FrameLoader::init()
<!DOCTYPE html>
<html>
<body>
<div id="host">
<div>F</div>
</div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host">
<div>A</div>
<div>B</div>
<div>C</div>
<div>D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot = new WebKitShadowRoot(host);
shadowRoot.innerHTML = '<content select="div[title]"></content>';
setTimeout(function() {
F.title = 'something';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
<!DOCTYPE html>
<html>
<body>
<div id="host">
<div>A</div>
<div>D</div>
<div>F</div>
</div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host">
<div class="selected">A</div>
<div>B</div>
<div>C</div>
<div class="selected">D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot = new WebKitShadowRoot(host);
shadowRoot.innerHTML = '<content select=".selected"></content>';
setTimeout(function() {
F.className = 'selected';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
<!DOCTYPE html>
<html>
<body>
<div id="host">
<div>F</div>
</div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host">
<div id="selected">A</div>
<div>B</div>
<div>C</div>
<div>D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot = new WebKitShadowRoot(host);
shadowRoot.innerHTML = '<content select="#selected"></content>';
setTimeout(function() {
A = selected;
F.id = 'selected';
A.id = '';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
<!DOCTYPE html>
<html>
<body>
<div id="host"><div>
<div>F</div>
</div></div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host1">
<div>A</div>
<div>B</div>
<div>C</div>
<div>D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot1 = new WebKitShadowRoot(host1);
shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
var host2 = shadowRoot1.getElementById('host2');
var shadowRoot2 = new WebKitShadowRoot(host2);
shadowRoot2.innerHTML = '<content select="div[title=selected]"></content>';
setTimeout(function() {
F.title = 'selected';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
<!DOCTYPE html>
<html>
<body>
<div id="host"><div>
<div>A</div>
<div>D</div>
<div>F</div>
</div></div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host1">
<div class="selected">A</div>
<div>B</div>
<div>C</div>
<div class="selected">D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot1 = new WebKitShadowRoot(host1);
shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
var host2 = shadowRoot1.getElementById('host2');
var shadowRoot2 = new WebKitShadowRoot(host2);
shadowRoot2.innerHTML = '<content select=".selected"></content>';
setTimeout(function() {
F.className = 'selected';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
<!DOCTYPE html>
<html>
<body>
<div id="host"><div>
<div>F</div>
</div></div>
</html>
<!DOCTYPE html>
<html>
<body>
<script src="resources/polyfill.js"></script>
<div id="host1">
<div id="selected">A</div>
<div>B</div>
<div>C</div>
<div>D</div>
<div>E</div>
<div id="F">F</div>
</div>
<script>
if (window.testRunner)
testRunner.waitUntilDone();
var shadowRoot1 = new WebKitShadowRoot(host1);
shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
var host2 = shadowRoot1.getElementById('host2');
var shadowRoot2 = new WebKitShadowRoot(host2);
shadowRoot2.innerHTML = '<content select="#selected"></content>';
setTimeout(function() {
A = selected;
A.id = '';
F.id = 'selected';
if (window.testRunner)
testRunner.notifyDone();
}, 0);
</script>
</html>
2012-11-19 Shinya Kawanaka <shinyak@chromium.org>
Changing id, className, or attribute should invalidate distribution
https://bugs.webkit.org/show_bug.cgi?id=100738
Reviewed by Dimitri Glazkov.
Added test code to modify id/class/attribute.
* DOM/ModifyAttribute.html: Added.
* DOM/resources/dom-perf/modify-attribute.js: Added.
(ModifyAttribute.CreateElementToSetUp):
(ModifyAttribute.ModifyId):
(ModifyAttribute.ModifyClass):
(ModifyAttribute.ModifyTitle):
2012-11-14 Ryosuke Niwa <rniwa@webkit.org>
runCount in runner.js should be renamed to iterationCount
......
<!DOCTYPE html>
<html>
<body>
<div id="container"><span id="benchmark_content"></span></div>
<script type="text/javascript" src="../resources/runner.js"></script>
<script type="text/javascript" src="resources/dom-perf.js"></script>
<script type="text/javascript" src="resources/dom-perf/modify-attribute.js"></script>
<script>runBenchmarkSuite(ModifyAttributeTest);</script>
</body>
</html>
/*
* Copyright (C) 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
* met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following disclaimer
* in the documentation and/or other materials provided with the
* distribution.
* * Neither the name of Google Inc. nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
// ModifyAttribute - measure attribute modification performance
var ModifyAttribute = {};
var nLoops = 500000;
ModifyAttribute.CreateElementToSetUp = function() {
return document.createElement('div');
}
ModifyAttribute.ModifyId = function(element) {
var nLoops = window.nLoops;
var idValue1 = 'id1';
var idValue2 = 'id2';
for (var i = 0; i < nLoops; ++i) {
element.id = idValue1;
element.id = idValue2;
}
}
ModifyAttribute.ModifyClass = function(element) {
var nLoops = window.nLoops;
var className1 = 'class1';
var className2 = 'class2';
for (var i = 0; i < nLoops; ++i) {
element.className = className1;
element.className = className2;
}
}
ModifyAttribute.ModifyTitle = function(element) {
var nLoops = window.nLoops;
var title1 = 'title1';
var title2 = 'title2';
for (var i = 0; i < nLoops; ++i) {
element.title = title1;
element.title = title2;
}
}
var ModifyAttributeTest = new BenchmarkSuite('ModifyAttribute', [
new Benchmark("Modify id", ModifyAttribute.ModifyId, ModifyAttribute.CreateElementToSetUp),
new Benchmark("Modify class", ModifyAttribute.ModifyClass, ModifyAttribute.CreateElementToSetUp),
new Benchmark("Modify title", ModifyAttribute.ModifyTitle, ModifyAttribute.CreateElementToSetUp)
]);
2012-11-19 Shinya Kawanaka <shinyak@chromium.org>
Changing id, className, or attribute should invalidate distribution
https://bugs.webkit.org/show_bug.cgi?id=100738
Reviewed by Dimitri Glazkov.
When id, className, or attribute is changed, we might have to invalidate distribution.
However, we don't want to do useless invalidation. So we consult with the RuleFeatureSet of ElementShadow
to invalidate distribution only if necessary.
For the code that className is changed, we can share a lot of code between invalidating distribution and
invalidating style. So we made checkNeedsStyleInvalidationForClassChange a template method, and share it.
Since attributeChanged() is a hot method, we don't want to make it slow. So we made one function to determine
whether we have to invalidate distribution, and make it called only if necessary. Also, we've optimized
shadowOfParentForDistribution() by making isInsertionPoint() de-virtualed. We consuded NodeFlags (IsInsertionPointFlag)
for this purpose.
We've measured how this patch makes changing attribute slow. I've measured each code 3 times.
DOM/ModifyAttribute.html is a micro benchmark which changes attribute a lot of times. The result of this benchmark
will be the most affected by this patch. However, it's only 2% performance regression.
DOM/ModifyAttribute.html
Before this patch:
median stdev min max [ms]
1st 494.0 3.36 490.0 502.0
2nd 503.5 3.44 497.0 512.0
3rd 494.0 3.48 488.0 499.0
After this patch:
median stdev min max [ms]
1st 504.0 2.00 501.0 509.0
2nd 505.5 3.08 500.0 513.0
3rd 507.0 2.32 502.0 510.0
Tests: fast/dom/shadow/distribution-attribute-modified.html
fast/dom/shadow/distribution-className-modified.html
fast/dom/shadow/distribution-id-modified.html
fast/dom/shadow/reprojection-attribute-modified.html
fast/dom/shadow/reprojection-className-modified.html
fast/dom/shadow/reprojection-id-modified.html
* dom/Element.cpp:
(WebCore::Element::attributeChanged):
(WebCore::HasSelectorForClassStyleFunctor::HasSelectorForClassStyleFunctor):
(HasSelectorForClassStyleFunctor):
(WebCore::HasSelectorForClassStyleFunctor::operator()): Returns true if StyleResolver::hasSelectorForClass returns true.
(WebCore):
(WebCore::HasSelectorForClassDistributionFunctor::HasSelectorForClassDistributionFunctor):
(HasSelectorForClassDistributionFunctor):
(WebCore::HasSelectorForClassDistributionFunctor::operator()): Returns true if ElementShadow::hasSelectForClass returns true.
(WebCore::checkFunctorForClassChange):
(WebCore::checkNeedsStyleInvalidationForClassChange):
(WebCore::checkNeedsDistributionInvalidationForClassChange): Extracted the implementation to checkFunctorForClassChange.
(WebCore::Element::shouldInvalidateDistributionWhenAttributeChanged):
* dom/Element.h:
(Element):
* dom/Node.h:
(WebCore::Node::isInsertionPoint):
* html/HTMLElement.h:
(HTMLElement):
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::InsertionPoint):
* html/shadow/InsertionPoint.h:
(InsertionPoint):
(WebCore::isInsertionPoint):
(WebCore::shadowOfParentForDistribution):
(WebCore::resolveReprojection):
2012-11-19 Nate Chapin <japhet@chromium.org>
Move empty loading to DocumentLoader, simplify FrameLoader::init()
......@@ -53,6 +53,7 @@
#include "HTMLOptionsCollection.h"
#include "HTMLParserIdioms.h"
#include "HTMLTableRowsCollection.h"
#include "InsertionPoint.h"
#include "InspectorInstrumentation.h"
#include "MutationObserverInterestGroup.h"
#include "MutationRecord.h"
......@@ -758,6 +759,11 @@ static bool checkNeedsStyleInvalidationForIdChange(const AtomicString& oldId, co
void Element::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
{
if (ElementShadow* parentElementShadow = shadowOfParentForDistribution(this)) {
if (shouldInvalidateDistributionWhenAttributeChanged(parentElementShadow, name, newValue))
parentElementShadow->invalidateDistribution();
}
parseAttribute(name, newValue);
document()->incDOMTreeVersion();
......@@ -818,21 +824,49 @@ static inline bool classStringHasClassName(const AtomicString& newClassString)
return classStringHasClassName(newClassString.characters16(), length);
}
static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& changedClasses, StyleResolver* styleResolver)
struct HasSelectorForClassStyleFunctor {
explicit HasSelectorForClassStyleFunctor(StyleResolver* resolver)
: styleResolver(resolver)
{ }
bool operator()(const AtomicString& className) const
{
return styleResolver->hasSelectorForClass(className);
}
StyleResolver* styleResolver;
};
struct HasSelectorForClassDistributionFunctor {
explicit HasSelectorForClassDistributionFunctor(ElementShadow* elementShadow)
: elementShadow(elementShadow)
{ }
bool operator()(const AtomicString& className) const
{
return elementShadow->selectRuleFeatureSet().hasSelectorForClass(className);
}
ElementShadow* elementShadow;
};
template<typename Functor>
static bool checkFunctorForClassChange(const SpaceSplitString& changedClasses, Functor functor)
{
unsigned changedSize = changedClasses.size();
for (unsigned i = 0; i < changedSize; ++i) {
if (styleResolver->hasSelectorForClass(changedClasses[i]))
if (functor(changedClasses[i]))
return true;
}
return false;
}
static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, StyleResolver* styleResolver)
template<typename Functor>
static bool checkFunctorForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, Functor functor)
{
unsigned oldSize = oldClasses.size();
if (!oldSize)
return checkNeedsStyleInvalidationForClassChange(newClasses, styleResolver);
return checkFunctorForClassChange(newClasses, functor);
BitVector remainingClassBits;
remainingClassBits.ensureSize(oldSize);
// Class vectors tend to be very short. This is faster than using a hash table.
......@@ -844,19 +878,39 @@ static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& ol
continue;
}
}
if (styleResolver->hasSelectorForClass(newClasses[i]))
if (functor(newClasses[i]))
return true;
}
for (unsigned i = 0; i < oldSize; ++i) {
// If the bit is not set the the corresponding class has been removed.
if (remainingClassBits.quickGet(i))
continue;
if (styleResolver->hasSelectorForClass(oldClasses[i]))
if (functor(oldClasses[i]))
return true;
}
return false;
}
static inline bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& changedClasses, StyleResolver* styleResolver)
{
return checkFunctorForClassChange(changedClasses, HasSelectorForClassStyleFunctor(styleResolver));
}
static inline bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, StyleResolver* styleResolver)
{
return checkFunctorForClassChange(oldClasses, newClasses, HasSelectorForClassStyleFunctor(styleResolver));
}
static inline bool checkNeedsDistributionInvalidationForClassChange(const SpaceSplitString& changedClasses, ElementShadow* elementShadow)
{
return checkFunctorForClassChange(changedClasses, HasSelectorForClassDistributionFunctor(elementShadow));
}
static inline bool checkNeedsDistributionInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, ElementShadow* elementShadow)
{
return checkFunctorForClassChange(oldClasses, newClasses, HasSelectorForClassDistributionFunctor(elementShadow));
}
void Element::classAttributeChanged(const AtomicString& newClassString)
{
StyleResolver* styleResolver = document()->styleResolverIfExists();
......@@ -886,6 +940,41 @@ void Element::classAttributeChanged(const AtomicString& newClassString)
setNeedsStyleRecalc();
}
bool Element::shouldInvalidateDistributionWhenAttributeChanged(ElementShadow* elementShadow, const QualifiedName& name, const AtomicString& newValue)
{
ASSERT(elementShadow);
elementShadow->ensureSelectFeatureSetCollected();
if (isIdAttributeName(name)) {
AtomicString oldId = attributeData()->idForStyleResolution();
AtomicString newId = makeIdForStyleResolution(newValue, document()->inQuirksMode());
if (newId != oldId) {
if (!oldId.isEmpty() && elementShadow->selectRuleFeatureSet().hasSelectorForId(oldId))
return true;
if (!newId.isEmpty() && elementShadow->selectRuleFeatureSet().hasSelectorForId(newId))
return true;
}
}
if (name == HTMLNames::classAttr) {
const AtomicString& newClassString = newValue;