Diff
Modified: trunk/Source/WebCore/ChangeLog (227786 => 227787)
--- trunk/Source/WebCore/ChangeLog 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/ChangeLog 2018-01-30 16:13:15 UTC (rev 227787)
@@ -1,3 +1,58 @@
+2018-01-30 Antti Koivisto <[email protected]>
+
+ Avoid traversing too much when doing class change invalidation
+ https://bugs.webkit.org/show_bug.cgi?id=181604
+
+ Reviewed by Zalan Bujtas.
+
+ We are now collecting information about which part of the tree a change in class can potentially affect.
+ Use the information to traverse only the required elements in Style::Invalidator.
+
+ The same mechanism can be later used for attribute and id change invalidation.
+
+ * css/DocumentRuleSets.cpp:
+ (WebCore::DocumentRuleSets::collectFeatures const):
+ (WebCore::DocumentRuleSets::classInvalidationRuleSets const):
+ (WebCore::DocumentRuleSets::subjectClassRules const): Deleted.
+ (WebCore::DocumentRuleSets::ancestorClassRules const): Deleted.
+
+ Remove separate subject and ancestor invalidation RuleSets. Instead collect all invalidation rulesets
+ to a vector along with their MatchElements.
+
+ * css/DocumentRuleSets.h:
+ * css/RuleFeature.cpp:
+ (WebCore::RuleFeatureSet::computeNextMatchElement):
+ (WebCore::RuleFeatureSet::computeSubSelectorMatchElement):
+ (WebCore::RuleFeatureSet::collectFeatures):
+
+ Similarly collect all class invalidation RuleFeatures to a general HashMap along with the MatchElement.
+
+ (WebCore::RuleFeatureSet::add):
+ (WebCore::RuleFeatureSet::clear):
+ (WebCore::RuleFeatureSet::shrinkToFit):
+ * css/RuleFeature.h:
+ (WebCore::RuleFeature::RuleFeature):
+ * style/ClassChangeInvalidation.cpp:
+ (WebCore::Style::ClassChangeInvalidation::computeInvalidation):
+
+ Find out InvalidationRuleSets to use.
+
+ (WebCore::Style::ClassChangeInvalidation::invalidateStyleWithRuleSets):
+
+ Pass them to Style::Invalidator.
+
+ * style/ClassChangeInvalidation.h:
+ * style/StyleInvalidator.cpp:
+ (WebCore::Style::Invalidator::invalidateStyleForTree):
+ (WebCore::Style::Invalidator::invalidateStyleForDescendants):
+ (WebCore::Style::Invalidator::invalidateStyleWithMatchElement):
+
+ Traverse only the part of the tree needed by the given MatchElement.
+
+ * style/StyleInvalidator.h:
+ * style/StyleSharingResolver.cpp:
+ (WebCore::Style::SharingResolver::classNamesAffectedByRules const):
+
2018-01-30 Javier Fernandez <[email protected]>
[css-align] The 'baseline' value must be invalid for the 'justify-content' property
Modified: trunk/Source/WebCore/css/DocumentRuleSets.cpp (227786 => 227787)
--- trunk/Source/WebCore/css/DocumentRuleSets.cpp 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/css/DocumentRuleSets.cpp 2018-01-30 16:13:15 UTC (rev 227787)
@@ -167,29 +167,34 @@
m_siblingRuleSet = makeRuleSet(m_features.siblingRules);
m_uncommonAttributeRuleSet = makeRuleSet(m_features.uncommonAttributeRules);
- m_subjectClassRuleSets.clear();
- m_ancestorClassRuleSets.clear();
+ m_classInvalidationRuleSets.clear();
m_ancestorAttributeRuleSetsForHTML.clear();
m_features.shrinkToFit();
}
-RuleSet* DocumentRuleSets::subjectClassRules(const AtomicString& className) const
+const Vector<InvalidationRuleSet>* DocumentRuleSets::classInvalidationRuleSets(const AtomicString& className) const
{
- return m_subjectClassRuleSets.ensure(className, [&] {
- auto* rules = m_features.subjectClassRules.get(className);
- return rules ? makeRuleSet(*rules) : nullptr;
+ return m_classInvalidationRuleSets.ensure(className, [&] () -> std::unique_ptr<Vector<InvalidationRuleSet>> {
+ auto* features = m_features.classRules.get(className);
+ if (!features)
+ return nullptr;
+ std::array<std::unique_ptr<RuleSet>, matchElementCount> matchElementArray;
+ for (auto& feature : *features) {
+ auto& ruleSet = matchElementArray[static_cast<unsigned>(*feature.matchElement)];
+ if (!ruleSet)
+ ruleSet = std::make_unique<RuleSet>();
+ ruleSet->addRule(feature.rule, feature.selectorIndex);
+ }
+ auto invalidationRuleSets = std::make_unique<Vector<InvalidationRuleSet>>();
+ for (unsigned i = 0; i < matchElementArray.size(); ++i) {
+ if (matchElementArray[i])
+ invalidationRuleSets->append({ static_cast<MatchElement>(i), WTFMove(matchElementArray[i]) });
+ }
+ return invalidationRuleSets;
}).iterator->value.get();
}
-RuleSet* DocumentRuleSets::ancestorClassRules(const AtomicString& className) const
-{
- return m_ancestorClassRuleSets.ensure(className, [&] {
- auto* rules = m_features.ancestorClassRules.get(className);
- return rules ? makeRuleSet(*rules) : nullptr;
- }).iterator->value.get();
-}
-
const DocumentRuleSets::AttributeRules* DocumentRuleSets::ancestorAttributeRulesForHTML(const AtomicString& attributeName) const
{
auto addResult = m_ancestorAttributeRuleSetsForHTML.add(attributeName, nullptr);
Modified: trunk/Source/WebCore/css/DocumentRuleSets.h (227786 => 227787)
--- trunk/Source/WebCore/css/DocumentRuleSets.h 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/css/DocumentRuleSets.h 2018-01-30 16:13:15 UTC (rev 227787)
@@ -38,6 +38,13 @@
class InspectorCSSOMWrappers;
class MediaQueryEvaluator;
+struct InvalidationRuleSet {
+ MatchElement matchElement;
+ std::unique_ptr<RuleSet> ruleSet;
+
+ WTF_MAKE_FAST_ALLOCATED;
+};
+
class DocumentRuleSets {
public:
DocumentRuleSets(StyleResolver&);
@@ -50,9 +57,9 @@
const RuleFeatureSet& features() const;
RuleSet* sibling() const { return m_siblingRuleSet.get(); }
RuleSet* uncommonAttribute() const { return m_uncommonAttributeRuleSet.get(); }
- RuleSet* subjectClassRules(const AtomicString& className) const;
- RuleSet* ancestorClassRules(const AtomicString& className) const;
+ const Vector<InvalidationRuleSet>* classInvalidationRuleSets(const AtomicString& className) const;
+
struct AttributeRules {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -91,8 +98,7 @@
mutable unsigned m_userAgentMediaQueryRuleCountOnUpdate { 0 };
mutable std::unique_ptr<RuleSet> m_siblingRuleSet;
mutable std::unique_ptr<RuleSet> m_uncommonAttributeRuleSet;
- mutable HashMap<AtomicString, std::unique_ptr<RuleSet>> m_subjectClassRuleSets;
- mutable HashMap<AtomicString, std::unique_ptr<RuleSet>> m_ancestorClassRuleSets;
+ mutable HashMap<AtomicString, std::unique_ptr<Vector<InvalidationRuleSet>>> m_classInvalidationRuleSets;
mutable HashMap<AtomicString, std::unique_ptr<AttributeRules>> m_ancestorAttributeRuleSetsForHTML;
};
Modified: trunk/Source/WebCore/css/RuleFeature.cpp (227786 => 227787)
--- trunk/Source/WebCore/css/RuleFeature.cpp 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/css/RuleFeature.cpp 2018-01-30 16:13:15 UTC (rev 227787)
@@ -35,7 +35,7 @@
namespace WebCore {
-RuleFeatureSet::MatchElement RuleFeatureSet::computeNextMatchElement(MatchElement matchElement, CSSSelector::RelationType relation)
+MatchElement RuleFeatureSet::computeNextMatchElement(MatchElement matchElement, CSSSelector::RelationType relation)
{
if (matchElement == MatchElement::Subject || matchElement == MatchElement::IndirectSibling || matchElement == MatchElement::DirectSibling) {
switch (relation) {
@@ -69,7 +69,7 @@
return matchElement;
};
-RuleFeatureSet::MatchElement RuleFeatureSet::computeSubSelectorMatchElement(MatchElement matchElement, const CSSSelector& selector)
+MatchElement RuleFeatureSet::computeSubSelectorMatchElement(MatchElement matchElement, const CSSSelector& selector)
{
ASSERT(selector.selectorList());
@@ -157,28 +157,12 @@
siblingRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
if (ruleData.containsUncommonAttributeSelector())
uncommonAttributeRules.append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
- for (auto& classNameAndMatchElement : selectorFeatures.classes) {
- auto& className = classNameAndMatchElement.first;
- switch (classNameAndMatchElement.second) {
- case MatchElement::Subject:
- subjectClassRules.ensure(className, [] {
- return std::make_unique<Vector<RuleFeature>>();
- }).iterator->value->append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
- break;
- case MatchElement::Parent:
- case MatchElement::Ancestor:
- ancestorClassRules.ensure(className, [] {
- return std::make_unique<Vector<RuleFeature>>();
- }).iterator->value->append(RuleFeature(ruleData.rule(), ruleData.selectorIndex()));
- break;
- case MatchElement::DirectSibling:
- case MatchElement::IndirectSibling:
- case MatchElement::ParentSibling:
- case MatchElement::AncestorSibling:
- case MatchElement::Host:
- otherClassesInRules.add(className);
- break;
- };
+ for (auto& nameAndMatch : selectorFeatures.classes) {
+ classRules.ensure(nameAndMatch.first, [] {
+ return std::make_unique<Vector<RuleFeature>>();
+ }).iterator->value->append(RuleFeature(ruleData.rule(), ruleData.selectorIndex(), nameAndMatch.second));
+ if (nameAndMatch.second == MatchElement::Host)
+ classesAffectingHost.add(nameAndMatch.first);
}
for (auto* selector : selectorFeatures.attributeSelectorsMatchingAncestors) {
// Hashing by attributeCanonicalLocalName makes this HTML specific.
@@ -196,21 +180,16 @@
{
idsInRules.add(other.idsInRules.begin(), other.idsInRules.end());
idsMatchingAncestorsInRules.add(other.idsMatchingAncestorsInRules.begin(), other.idsMatchingAncestorsInRules.end());
- otherClassesInRules.add(other.otherClassesInRules.begin(), other.otherClassesInRules.end());
attributeCanonicalLocalNamesInRules.add(other.attributeCanonicalLocalNamesInRules.begin(), other.attributeCanonicalLocalNamesInRules.end());
attributeLocalNamesInRules.add(other.attributeLocalNamesInRules.begin(), other.attributeLocalNamesInRules.end());
siblingRules.appendVector(other.siblingRules);
uncommonAttributeRules.appendVector(other.uncommonAttributeRules);
- for (auto& keyValuePair : other.ancestorClassRules) {
- ancestorClassRules.ensure(keyValuePair.key, [] {
+ for (auto& keyValuePair : other.classRules) {
+ classRules.ensure(keyValuePair.key, [] {
return std::make_unique<Vector<RuleFeature>>();
}).iterator->value->appendVector(*keyValuePair.value);
}
- for (auto& keyValuePair : other.subjectClassRules) {
- subjectClassRules.ensure(keyValuePair.key, [] {
- return std::make_unique<Vector<RuleFeature>>();
- }).iterator->value->appendVector(*keyValuePair.value);
- }
+ classesAffectingHost.add(other.classesAffectingHost.begin(), other.classesAffectingHost.end());
for (auto& keyValuePair : other.ancestorAttributeRulesForHTML) {
auto addResult = ancestorAttributeRulesForHTML.ensure(keyValuePair.key, [] {
@@ -229,13 +208,12 @@
{
idsInRules.clear();
idsMatchingAncestorsInRules.clear();
- otherClassesInRules.clear();
attributeCanonicalLocalNamesInRules.clear();
attributeLocalNamesInRules.clear();
siblingRules.clear();
uncommonAttributeRules.clear();
- ancestorClassRules.clear();
- subjectClassRules.clear();
+ classRules.clear();
+ classesAffectingHost.clear();
ancestorAttributeRulesForHTML.clear();
usesFirstLineRules = false;
usesFirstLetterRules = false;
@@ -245,10 +223,8 @@
{
siblingRules.shrinkToFit();
uncommonAttributeRules.shrinkToFit();
- for (auto& rules : ancestorClassRules.values())
+ for (auto& rules : classRules.values())
rules->shrinkToFit();
- for (auto& rules : subjectClassRules.values())
- rules->shrinkToFit();
for (auto& rules : ancestorAttributeRulesForHTML.values())
rules->features.shrinkToFit();
}
Modified: trunk/Source/WebCore/css/RuleFeature.h (227786 => 227787)
--- trunk/Source/WebCore/css/RuleFeature.h 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/css/RuleFeature.h 2018-01-30 16:13:15 UTC (rev 227787)
@@ -33,14 +33,19 @@
class RuleData;
class StyleRule;
+enum class MatchElement { Subject, Parent, Ancestor, DirectSibling, IndirectSibling, ParentSibling, AncestorSibling, Host };
+constexpr unsigned matchElementCount = static_cast<unsigned>(MatchElement::Host) + 1;
+
struct RuleFeature {
- RuleFeature(StyleRule* rule, unsigned selectorIndex)
+ RuleFeature(StyleRule* rule, unsigned selectorIndex, std::optional<MatchElement> matchElement = std::nullopt)
: rule(rule)
, selectorIndex(selectorIndex)
+ , matchElement(matchElement)
{
}
StyleRule* rule;
unsigned selectorIndex;
+ std::optional<MatchElement> matchElement;
};
struct RuleFeatureSet {
@@ -55,9 +60,9 @@
HashSet<AtomicString> attributeLocalNamesInRules;
Vector<RuleFeature> siblingRules;
Vector<RuleFeature> uncommonAttributeRules;
- HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>> ancestorClassRules;
- HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>> subjectClassRules;
- HashSet<AtomicString> otherClassesInRules;
+
+ HashMap<AtomicString, std::unique_ptr<Vector<RuleFeature>>> classRules;
+ HashSet<AtomicString> classesAffectingHost;
struct AttributeRules {
WTF_MAKE_FAST_ALLOCATED;
@@ -71,8 +76,6 @@
bool usesFirstLetterRules { false };
private:
- enum class MatchElement { Subject, Parent, Ancestor, DirectSibling, IndirectSibling, ParentSibling, AncestorSibling, Host };
-
static MatchElement computeNextMatchElement(MatchElement, CSSSelector::RelationType);
static MatchElement computeSubSelectorMatchElement(MatchElement, const CSSSelector&);
Modified: trunk/Source/WebCore/style/ClassChangeInvalidation.cpp (227786 => 227787)
--- trunk/Source/WebCore/style/ClassChangeInvalidation.cpp 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/style/ClassChangeInvalidation.cpp 2018-01-30 16:13:15 UTC (rev 227787)
@@ -89,26 +89,13 @@
bool shouldInvalidateCurrent = false;
bool mayAffectStyleInShadowTree = false;
- ClassChangeVector classesAffectingDescendant;
- ClassChangeVector classesAffectingCurrent;
traverseRuleFeatures(m_element, [&] (const RuleFeatureSet& features, bool mayAffectShadowTree) {
for (auto* changedClass : changedClasses) {
- bool mayAffectStyle = false;
- if (features.otherClassesInRules.contains(changedClass)) {
+ if (mayAffectShadowTree && features.classRules.contains(changedClass))
+ mayAffectStyleInShadowTree = true;
+ if (features.classesAffectingHost.contains(changedClass))
shouldInvalidateCurrent = true;
- mayAffectStyle = true;
- }
- if (features.ancestorClassRules.contains(changedClass)) {
- classesAffectingDescendant.append(changedClass);
- mayAffectStyle = true;
- }
- if (features.subjectClassRules.contains(changedClass)) {
- classesAffectingCurrent.append(changedClass);
- mayAffectStyle = true;
- }
- if (mayAffectStyle && mayAffectShadowTree)
- mayAffectStyleInShadowTree = true;
}
});
@@ -118,31 +105,24 @@
return;
}
+ if (shouldInvalidateCurrent)
+ m_element.invalidateStyle();
+
auto& ruleSets = m_element.styleResolver().ruleSets();
- if (childrenOfType<Element>(m_element).first()) {
- for (auto* changedClass : classesAffectingDescendant) {
- if (auto* rules = ruleSets.ancestorClassRules(changedClass))
- m_invalidationRuleSets.append(rules);
+ for (auto* changedClass : changedClasses) {
+ if (auto* invalidationRuleSets = ruleSets.classInvalidationRuleSets(changedClass)) {
+ for (auto& invalidationRuleSet : *invalidationRuleSets)
+ m_invalidationRuleSets.append(&invalidationRuleSet);
}
}
-
- if (shouldInvalidateCurrent) {
- m_element.invalidateStyle();
- return;
- }
-
- for (auto* changedClass : classesAffectingCurrent) {
- if (auto* rules = ruleSets.subjectClassRules(changedClass))
- m_invalidationRuleSets.append(rules);
- }
}
void ClassChangeInvalidation::invalidateStyleWithRuleSets()
{
- for (auto* rules : m_invalidationRuleSets) {
- Invalidator invalidator(*rules);
- invalidator.invalidateStyle(m_element);
+ for (auto* invalidationRuleSet : m_invalidationRuleSets) {
+ Invalidator invalidator(*invalidationRuleSet->ruleSet);
+ invalidator.invalidateStyleWithMatchElement(m_element, invalidationRuleSet->matchElement);
}
}
Modified: trunk/Source/WebCore/style/ClassChangeInvalidation.h (227786 => 227787)
--- trunk/Source/WebCore/style/ClassChangeInvalidation.h 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/style/ClassChangeInvalidation.h 2018-01-30 16:13:15 UTC (rev 227787)
@@ -33,6 +33,7 @@
class DocumentRuleSets;
class RuleSet;
class SpaceSplitString;
+struct InvalidationRuleSet;
namespace Style {
@@ -48,7 +49,7 @@
const bool m_isEnabled;
Element& m_element;
- Vector<const RuleSet*, 4> m_invalidationRuleSets;
+ Vector<const InvalidationRuleSet*, 4> m_invalidationRuleSets;
};
inline ClassChangeInvalidation::ClassChangeInvalidation(Element& element, const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses)
Modified: trunk/Source/WebCore/style/StyleInvalidator.cpp (227786 => 227787)
--- trunk/Source/WebCore/style/StyleInvalidator.cpp 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/style/StyleInvalidator.cpp 2018-01-30 16:13:15 UTC (rev 227787)
@@ -143,7 +143,11 @@
{
if (invalidateIfNeeded(root, filter) == CheckDescendants::No)
return;
+ invalidateStyleForDescendants(root, filter);
+}
+void Invalidator::invalidateStyleForDescendants(Element& root, SelectorFilter* filter)
+{
Vector<Element*, 20> parentStack;
Element* previousElement = &root;
auto descendants = descendantsOfType<Element>(root);
@@ -205,5 +209,35 @@
invalidateStyleForTree(element, nullptr);
}
+void Invalidator::invalidateStyleWithMatchElement(Element& element, MatchElement matchElement)
+{
+ switch (matchElement) {
+ case MatchElement::Subject: {
+ invalidateIfNeeded(element, nullptr);
+ break;
+ }
+ case MatchElement::Parent: {
+ auto children = childrenOfType<Element>(element);
+ for (auto& child : children)
+ invalidateIfNeeded(child, nullptr);
+ break;
+ }
+ case MatchElement::Ancestor: {
+ invalidateStyleForDescendants(element, nullptr);
+ break;
+ }
+ case MatchElement::DirectSibling:
+ case MatchElement::IndirectSibling:
+ case MatchElement::ParentSibling:
+ case MatchElement::AncestorSibling:
+ // FIXME: Currently sibling invalidation happens during style resolution tree walk. It should be done here too.
+ element.invalidateStyle();
+ break;
+ case MatchElement::Host:
+ // FIXME: Handle this here as well.
+ break;
+ }
}
+
}
+}
Modified: trunk/Source/WebCore/style/StyleInvalidator.h (227786 => 227787)
--- trunk/Source/WebCore/style/StyleInvalidator.h 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/style/StyleInvalidator.h 2018-01-30 16:13:15 UTC (rev 227787)
@@ -25,6 +25,7 @@
#pragma once
+#include "RuleFeature.h"
#include <wtf/Forward.h>
namespace WebCore {
@@ -49,11 +50,13 @@
void invalidateStyle(Document&);
void invalidateStyle(ShadowRoot&);
void invalidateStyle(Element&);
+ void invalidateStyleWithMatchElement(Element&, MatchElement);
private:
enum class CheckDescendants { Yes, No };
CheckDescendants invalidateIfNeeded(Element&, const SelectorFilter*);
void invalidateStyleForTree(Element&, SelectorFilter*);
+ void invalidateStyleForDescendants(Element&, SelectorFilter*);
std::unique_ptr<RuleSet> m_ownedRuleSet;
const RuleSet& m_ruleSet;
Modified: trunk/Source/WebCore/style/StyleSharingResolver.cpp (227786 => 227787)
--- trunk/Source/WebCore/style/StyleSharingResolver.cpp 2018-01-30 16:03:14 UTC (rev 227786)
+++ trunk/Source/WebCore/style/StyleSharingResolver.cpp 2018-01-30 16:13:15 UTC (rev 227787)
@@ -340,13 +340,8 @@
bool SharingResolver::classNamesAffectedByRules(const SpaceSplitString& classNames) const
{
for (unsigned i = 0; i < classNames.size(); ++i) {
- if (m_ruleSets.features().otherClassesInRules.contains(classNames[i]))
+ if (m_ruleSets.features().classRules.contains(classNames[i]))
return true;
- if (m_ruleSets.features().ancestorClassRules.contains(classNames[i]))
- return true;
- if (m_ruleSets.features().subjectClassRules.contains(classNames[i]))
- return true;
-
}
return false;
}
