- Revision
- 287325
- Author
- an...@apple.com
- Date
- 2021-12-21 11:36:05 -0800 (Tue, 21 Dec 2021)
Log Message
REGRESSION(r286169): 0.3% Speedometer regression
https://bugs.webkit.org/show_bug.cgi?id=234549
Reviewed by Alan Bujtas.
There is some additional work in ChildChangeInvalidation even when :has is not used.
Speedemater2 Vanilla-* subtests are very heavy on DOM mutations.
* style/ChildChangeInvalidation.cpp:
(WebCore::Style::ChildChangeInvalidation::invalidateForHasBeforeMutation):
(WebCore::Style::ChildChangeInvalidation::invalidateForHasAfterMutation):
(WebCore::Style::ChildChangeInvalidation::traverseRemovedElements):
(WebCore::Style::ChildChangeInvalidation::traverseAddedElements):
(WebCore::Style::ChildChangeInvalidation::ChildChangeInvalidation): Deleted.
(WebCore::Style::ChildChangeInvalidation::~ChildChangeInvalidation): Deleted.
(WebCore::Style::needsTraversal): Deleted.
* style/ChildChangeInvalidation.h:
(WebCore::Style::ChildChangeInvalidation::ChildChangeInvalidation):
(WebCore::Style::ChildChangeInvalidation::~ChildChangeInvalidation):
Inline the constructor and destructor.
Compute the :has invalidation status only once.
* style/RuleFeature.h:
(WebCore::Style::RuleFeatureSet::usesHasPseudoClass const):
* style/StyleScope.cpp:
(WebCore::Style::Scope::resolver):
(WebCore::Style::Scope::updateActiveStyleSheets):
* style/StyleScope.h:
(WebCore::Style::Scope::usesStyleBasedEditability const):
(WebCore::Style::Scope::usesHasPseudoClass const):
Add a Scope bit for this for faster testing.
(WebCore::Style::Scope::usesStyleBasedEditability): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287324 => 287325)
--- trunk/Source/WebCore/ChangeLog 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/ChangeLog 2021-12-21 19:36:05 UTC (rev 287325)
@@ -1,3 +1,41 @@
+2021-12-21 Antti Koivisto <an...@apple.com>
+
+ REGRESSION(r286169): 0.3% Speedometer regression
+ https://bugs.webkit.org/show_bug.cgi?id=234549
+
+ Reviewed by Alan Bujtas.
+
+ There is some additional work in ChildChangeInvalidation even when :has is not used.
+ Speedemater2 Vanilla-* subtests are very heavy on DOM mutations.
+
+ * style/ChildChangeInvalidation.cpp:
+ (WebCore::Style::ChildChangeInvalidation::invalidateForHasBeforeMutation):
+ (WebCore::Style::ChildChangeInvalidation::invalidateForHasAfterMutation):
+ (WebCore::Style::ChildChangeInvalidation::traverseRemovedElements):
+ (WebCore::Style::ChildChangeInvalidation::traverseAddedElements):
+ (WebCore::Style::ChildChangeInvalidation::ChildChangeInvalidation): Deleted.
+ (WebCore::Style::ChildChangeInvalidation::~ChildChangeInvalidation): Deleted.
+ (WebCore::Style::needsTraversal): Deleted.
+ * style/ChildChangeInvalidation.h:
+ (WebCore::Style::ChildChangeInvalidation::ChildChangeInvalidation):
+ (WebCore::Style::ChildChangeInvalidation::~ChildChangeInvalidation):
+
+ Inline the constructor and destructor.
+ Compute the :has invalidation status only once.
+
+ * style/RuleFeature.h:
+ (WebCore::Style::RuleFeatureSet::usesHasPseudoClass const):
+ * style/StyleScope.cpp:
+ (WebCore::Style::Scope::resolver):
+ (WebCore::Style::Scope::updateActiveStyleSheets):
+ * style/StyleScope.h:
+ (WebCore::Style::Scope::usesStyleBasedEditability const):
+ (WebCore::Style::Scope::usesHasPseudoClass const):
+
+ Add a Scope bit for this for faster testing.
+
+ (WebCore::Style::Scope::usesStyleBasedEditability): Deleted.
+
2021-12-21 Sam Weinig <wei...@apple.com>
Gradient encode/decode does not validate that the serialized stops were sorted
Modified: trunk/Source/WebCore/style/ChildChangeInvalidation.cpp (287324 => 287325)
--- trunk/Source/WebCore/style/ChildChangeInvalidation.cpp 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/style/ChildChangeInvalidation.cpp 2021-12-21 19:36:05 UTC (rev 287325)
@@ -36,31 +36,6 @@
namespace WebCore::Style {
-ChildChangeInvalidation::ChildChangeInvalidation(ContainerNode& container, const ContainerNode::ChildChange& childChange)
- : m_parentElement(is<Element>(container) ? downcast<Element>(&container) : nullptr)
- , m_isEnabled(m_parentElement ? m_parentElement->needsStyleInvalidation() : false)
- , m_childChange(childChange)
-{
- if (!m_isEnabled)
- return;
-
- traverseRemovedElements([&](auto& changedElement) {
- invalidateForChangedElement(changedElement);
- });
-}
-
-ChildChangeInvalidation::~ChildChangeInvalidation()
-{
- if (!m_isEnabled)
- return;
-
- traverseAddedElements([&](auto& changedElement) {
- invalidateForChangedElement(changedElement);
- });
-
- invalidateAfterChange();
-}
-
void ChildChangeInvalidation::invalidateForChangedElement(Element& changedElement)
{
auto& ruleSets = parentElement().styleResolver().ruleSets();
@@ -108,19 +83,30 @@
Invalidator::invalidateWithMatchElementRuleSets(changedElement, matchElementRuleSets);
}
-static bool needsTraversal(const RuleFeatureSet& features, const ContainerNode::ChildChange& childChange)
+void ChildChangeInvalidation::invalidateForHasBeforeMutation()
{
- if (features.usesMatchElement(MatchElement::HasChild))
- return true;
- if (features.usesMatchElement(MatchElement::HasDescendant))
- return true;
- if (features.usesMatchElement(MatchElement::HasSiblingDescendant))
- return true;
- if (features.usesMatchElement(MatchElement::HasNonSubject))
- return true;
- return features.usesMatchElement(MatchElement::HasSibling) && childChange.previousSiblingElement;
-};
+ ASSERT(m_needsHasInvalidation);
+ if (m_childChange.isInsertion() && m_childChange.type != ContainerNode::ChildChange::Type::AllChildrenReplaced)
+ return;
+
+ traverseRemovedElements([&](auto& changedElement) {
+ invalidateForChangedElement(changedElement);
+ });
+}
+
+void ChildChangeInvalidation::invalidateForHasAfterMutation()
+{
+ ASSERT(m_needsHasInvalidation);
+
+ if (!m_childChange.isInsertion())
+ return;
+
+ traverseAddedElements([&](auto& changedElement) {
+ invalidateForChangedElement(changedElement);
+ });
+}
+
static bool needsDescendantTraversal(const RuleFeatureSet& features)
{
if (features.usesMatchElement(MatchElement::HasNonSubject))
@@ -131,13 +117,7 @@
template<typename Function>
void ChildChangeInvalidation::traverseRemovedElements(Function&& function)
{
- if (m_childChange.isInsertion() && m_childChange.type != ContainerNode::ChildChange::Type::AllChildrenReplaced)
- return;
-
auto& features = parentElement().styleResolver().ruleSets().features();
- if (!needsTraversal(features, m_childChange))
- return;
-
bool needsDescendantTraversal = Style::needsDescendantTraversal(features);
auto* toRemove = m_childChange.previousSiblingElement ? m_childChange.previousSiblingElement->nextElementSibling() : parentElement().firstElementChild();
@@ -155,9 +135,6 @@
template<typename Function>
void ChildChangeInvalidation::traverseAddedElements(Function&& function)
{
- if (!m_childChange.isInsertion())
- return;
-
auto* newElement = [&] {
auto* previous = m_childChange.previousSiblingElement;
auto* candidate = previous ? ElementTraversal::nextSibling(*previous) : ElementTraversal::firstChild(parentElement());
@@ -169,12 +146,9 @@
if (!newElement)
return;
- auto& features = parentElement().styleResolver().ruleSets().features();
- if (!needsTraversal(features, m_childChange))
- return;
-
function(*newElement);
+ auto& features = parentElement().styleResolver().ruleSets().features();
if (!needsDescendantTraversal(features))
return;
Modified: trunk/Source/WebCore/style/ChildChangeInvalidation.h (287324 => 287325)
--- trunk/Source/WebCore/style/ChildChangeInvalidation.h 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/style/ChildChangeInvalidation.h 2021-12-21 19:36:05 UTC (rev 287325)
@@ -27,6 +27,7 @@
#include "Element.h"
#include "StyleInvalidator.h"
+#include "StyleScope.h"
namespace WebCore {
namespace Style {
@@ -39,6 +40,8 @@
static void invalidateAfterFinishedParsingChildren(Element&);
private:
+ void invalidateForHasBeforeMutation();
+ void invalidateForHasAfterMutation();
void invalidateAfterChange();
void checkForSiblingStyleChanges();
void invalidateForChangedElement(Element&);
@@ -49,10 +52,35 @@
Element& parentElement() { return *m_parentElement; }
Element* m_parentElement { nullptr };
+ const ContainerNode::ChildChange& m_childChange;
+
const bool m_isEnabled;
-
- const ContainerNode::ChildChange& m_childChange;
+ const bool m_needsHasInvalidation;
};
+inline ChildChangeInvalidation::ChildChangeInvalidation(ContainerNode& container, const ContainerNode::ChildChange& childChange)
+ : m_parentElement(is<Element>(container) ? downcast<Element>(&container) : nullptr)
+ , m_childChange(childChange)
+ , m_isEnabled(m_parentElement ? m_parentElement->needsStyleInvalidation() : false)
+ , m_needsHasInvalidation(m_isEnabled && Scope::forNode(*m_parentElement).usesHasPseudoClass())
+{
+ if (!m_isEnabled)
+ return;
+
+ if (m_needsHasInvalidation)
+ invalidateForHasBeforeMutation();
}
+
+inline ChildChangeInvalidation::~ChildChangeInvalidation()
+{
+ if (!m_isEnabled)
+ return;
+
+ if (m_needsHasInvalidation)
+ invalidateForHasAfterMutation();
+
+ invalidateAfterChange();
}
+
+}
+}
Modified: trunk/Source/WebCore/style/RuleFeature.h (287324 => 287325)
--- trunk/Source/WebCore/style/RuleFeature.h 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/style/RuleFeature.h 2021-12-21 19:36:05 UTC (rev 287325)
@@ -85,6 +85,7 @@
void collectFeatures(const RuleData&);
void registerContentAttribute(const AtomString&);
+ bool usesHasPseudoClass() const;
bool usesMatchElement(MatchElement matchElement) const { return usedMatchElements[static_cast<uint8_t>(matchElement)]; }
void setUsesMatchElement(MatchElement matchElement) { usedMatchElements[static_cast<uint8_t>(matchElement)] = true; }
@@ -135,5 +136,14 @@
return static_cast<InvalidationKeyType>(std::get<1>(key)) == InvalidationKeyType::Universal;
}
+inline bool RuleFeatureSet::usesHasPseudoClass() const
+{
+ return usesMatchElement(MatchElement::HasChild)
+ || usesMatchElement(MatchElement::HasDescendant)
+ || usesMatchElement(MatchElement::HasSiblingDescendant)
+ || usesMatchElement(MatchElement::HasSibling)
+ || usesMatchElement(MatchElement::HasNonSubject);
+}
+
} // namespace Style
} // namespace WebCore
Modified: trunk/Source/WebCore/style/StyleScope.cpp (287324 => 287325)
--- trunk/Source/WebCore/style/StyleScope.cpp 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/style/StyleScope.cpp 2021-12-21 19:36:05 UTC (rev 287325)
@@ -85,6 +85,9 @@
createOrFindSharedShadowTreeResolver();
else
createDocumentResolver();
+
+ if (m_resolver->ruleSets().features().usesHasPseudoClass())
+ m_usesHasPseudoClass = true;
}
return *m_resolver;
}
@@ -506,6 +509,9 @@
m_usesStyleBasedEditability = true;
}
+ if (m_resolver && m_resolver->ruleSets().features().usesHasPseudoClass())
+ m_usesHasPseudoClass = true;
+
invalidateStyleAfterStyleSheetChange(styleSheetChange);
}
Modified: trunk/Source/WebCore/style/StyleScope.h (287324 => 287325)
--- trunk/Source/WebCore/style/StyleScope.h 2021-12-21 19:15:59 UTC (rev 287324)
+++ trunk/Source/WebCore/style/StyleScope.h 2021-12-21 19:36:05 UTC (rev 287325)
@@ -88,7 +88,8 @@
bool hasPendingSheetInBody(const Element&) const;
bool hasPendingSheet(const ProcessingInstruction&) const;
- bool usesStyleBasedEditability() { return m_usesStyleBasedEditability; }
+ bool usesStyleBasedEditability() const { return m_usesStyleBasedEditability; }
+ bool usesHasPseudoClass() const { return m_usesHasPseudoClass; }
bool activeStyleSheetsContains(const CSSStyleSheet*) const;
@@ -196,6 +197,7 @@
bool m_hasDescendantWithPendingUpdate { false };
bool m_usesStyleBasedEditability { false };
+ bool m_usesHasPseudoClass { false };
bool m_isUpdatingStyleResolver { false };
std::optional<MediaQueryViewportState> m_viewportStateOnPreviousMediaQueryEvaluation;
