- Revision
- 198584
- Author
- [email protected]
- Date
- 2016-03-23 07:16:17 -0700 (Wed, 23 Mar 2016)
Log Message
Share style by sharing RenderStyle substructures not the object itself
https://bugs.webkit.org/show_bug.cgi?id=155787
Reviewed by Anreas Kling.
The current approach where we share RenderStyle objects between elements leads to lot of awkward and bug-prone code.
Most of the RenderStyle consists of shareable substructures. It is better to just share those.
With this patch we create shared styles with RenderStyle::clone(). Sharing is traced as state in Style::SharingResolver
instead of relying on RenderStyle equality to locate potential sharing cousins.
* rendering/style/StyleRareNonInheritedData.cpp:
(WebCore::StyleRareNonInheritedData::operator==):
m_altText was missing from operator==
This was exposed by TreeResolver::resolveElement change, tested by fast/css/alt-inherit-initial.html
* style/StyleSharingResolver.cpp:
(WebCore::Style::elementHasDirectionAuto):
(WebCore::Style::SharingResolver::resolve):
Save share results to a map.
(WebCore::Style::SharingResolver::findSibling):
(WebCore::Style::SharingResolver::locateCousinList):
Instead of traversing we can now just do a hash lookup to locate a candidate cousin list.
There is no need for recursion anymore, the map covers sharing beyond immediate siblings too.
Remove most tests here as they have been already covered when sharing occured.
(WebCore::Style::canShareStyleWithControl):
* style/StyleSharingResolver.h:
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::styleForElement):
(WebCore::Style::TreeResolver::resolveElement):
No need to do forced setting anymore just to support style sharing.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (198583 => 198584)
--- trunk/Source/WebCore/ChangeLog 2016-03-23 14:05:54 UTC (rev 198583)
+++ trunk/Source/WebCore/ChangeLog 2016-03-23 14:16:17 UTC (rev 198584)
@@ -1,3 +1,43 @@
+2016-03-23 Antti Koivisto <[email protected]>
+
+ Share style by sharing RenderStyle substructures not the object itself
+ https://bugs.webkit.org/show_bug.cgi?id=155787
+
+ Reviewed by Anreas Kling.
+
+ The current approach where we share RenderStyle objects between elements leads to lot of awkward and bug-prone code.
+ Most of the RenderStyle consists of shareable substructures. It is better to just share those.
+
+ With this patch we create shared styles with RenderStyle::clone(). Sharing is traced as state in Style::SharingResolver
+ instead of relying on RenderStyle equality to locate potential sharing cousins.
+
+ * rendering/style/StyleRareNonInheritedData.cpp:
+ (WebCore::StyleRareNonInheritedData::operator==):
+
+ m_altText was missing from operator==
+ This was exposed by TreeResolver::resolveElement change, tested by fast/css/alt-inherit-initial.html
+
+ * style/StyleSharingResolver.cpp:
+ (WebCore::Style::elementHasDirectionAuto):
+ (WebCore::Style::SharingResolver::resolve):
+
+ Save share results to a map.
+
+ (WebCore::Style::SharingResolver::findSibling):
+ (WebCore::Style::SharingResolver::locateCousinList):
+
+ Instead of traversing we can now just do a hash lookup to locate a candidate cousin list.
+ There is no need for recursion anymore, the map covers sharing beyond immediate siblings too.
+ Remove most tests here as they have been already covered when sharing occured.
+
+ (WebCore::Style::canShareStyleWithControl):
+ * style/StyleSharingResolver.h:
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::styleForElement):
+ (WebCore::Style::TreeResolver::resolveElement):
+
+ No need to do forced setting anymore just to support style sharing.
+
2016-03-23 Gyuyoung Kim <[email protected]>
Reduce PassRefPtr uses in editing
Modified: trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp (198583 => 198584)
--- trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp 2016-03-23 14:05:54 UTC (rev 198583)
+++ trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp 2016-03-23 14:16:17 UTC (rev 198584)
@@ -244,6 +244,7 @@
#endif
&& contentDataEquivalent(o)
&& arePointingToEqualData(m_counterDirectives, o.m_counterDirectives)
+ && m_altText == o.m_altText
&& arePointingToEqualData(m_boxShadow, o.m_boxShadow)
&& arePointingToEqualData(m_willChange, o.m_willChange)
&& arePointingToEqualData(m_boxReflect, o.m_boxReflect)
Modified: trunk/Source/WebCore/style/StyleSharingResolver.cpp (198583 => 198584)
--- trunk/Source/WebCore/style/StyleSharingResolver.cpp 2016-03-23 14:05:54 UTC (rev 198583)
+++ trunk/Source/WebCore/style/StyleSharingResolver.cpp 2016-03-23 14:16:17 UTC (rev 198584)
@@ -42,7 +42,6 @@
namespace Style {
static const unsigned cStyleSearchThreshold = 10;
-static const unsigned cStyleSearchLevelThreshold = 10;
struct SharingResolver::Context {
const StyledElement& element;
@@ -68,7 +67,7 @@
return is<HTMLElement>(element) && downcast<HTMLElement>(element).hasDirectionAuto();
}
-const Element* SharingResolver::resolve(const Element& searchElement) const
+RefPtr<RenderStyle> SharingResolver::resolve(const Element& searchElement)
{
if (!is<StyledElement>(searchElement))
return nullptr;
@@ -103,14 +102,13 @@
// Check previous siblings and their cousins.
unsigned count = 0;
- unsigned visitedNodeCount = 0;
StyledElement* shareElement = nullptr;
Node* cousinList = element.previousSibling();
while (cousinList) {
shareElement = findSibling(context, cousinList, count);
if (shareElement)
break;
- cousinList = locateCousinList(cousinList->parentElement(), visitedNodeCount);
+ cousinList = locateCousinList(cousinList->parentElement());
}
// If we have exhausted all our budget or our cousins.
@@ -127,7 +125,9 @@
if (parentElementPreventsSharing(parentElement))
return nullptr;
- return shareElement;
+ m_elementsSharingStyle.add(&element, shareElement);
+
+ return RenderStyle::clone(shareElement->renderStyle());
}
StyledElement* SharingResolver::findSibling(const Context& context, Node* node, unsigned& count) const
@@ -143,47 +143,18 @@
return downcast<StyledElement>(node);
}
-Node* SharingResolver::locateCousinList(Element* parent, unsigned& visitedNodeCount) const
+Node* SharingResolver::locateCousinList(const Element* parent) const
{
- if (visitedNodeCount >= cStyleSearchThreshold * cStyleSearchLevelThreshold)
- return nullptr;
- if (!is<StyledElement>(parent))
- return nullptr;
- auto& styledParent = downcast<StyledElement>(*parent);
- if (styledParent.inlineStyle())
- return nullptr;
- if (is<SVGElement>(styledParent) && downcast<SVGElement>(styledParent).animatedSMILStyleProperties())
- return nullptr;
- if (styledParent.hasID() && m_ruleSets.features().idsInRules.contains(styledParent.idForStyleResolution().impl()))
- return nullptr;
-
- RenderStyle* parentStyle = styledParent.renderStyle();
- unsigned subcount = 0;
- Node* thisCousin = &styledParent;
- Node* currentNode = styledParent.previousSibling();
-
- // Reserve the tries for this level. This effectively makes sure that the algorithm
- // will never go deeper than cStyleSearchLevelThreshold levels into recursion.
- visitedNodeCount += cStyleSearchThreshold;
- while (thisCousin) {
- for (; currentNode; currentNode = currentNode->previousSibling()) {
- if (++subcount > cStyleSearchThreshold)
- return nullptr;
- if (!is<Element>(*currentNode))
- continue;
- auto& currentElement = downcast<Element>(*currentNode);
- if (currentElement.renderStyle() != parentStyle)
- continue;
- if (!currentElement.lastChild())
- continue;
- if (!parentElementPreventsSharing(currentElement)) {
- // Adjust for unused reserved tries.
- visitedNodeCount -= cStyleSearchThreshold - subcount;
- return currentNode->lastChild();
- }
+ const unsigned maximumSearchCount = 10;
+ for (unsigned count = 0; count < maximumSearchCount; ++count) {
+ auto* elementSharingParentStyle = m_elementsSharingStyle.get(parent);
+ if (!elementSharingParentStyle)
+ return nullptr;
+ if (!parentElementPreventsSharing(*elementSharingParentStyle)) {
+ if (auto* cousin = elementSharingParentStyle->lastChild())
+ return cousin;
}
- currentNode = locateCousinList(thisCousin->parentElement(), visitedNodeCount);
- thisCousin = currentNode;
+ parent = elementSharingParentStyle;
}
return nullptr;
Modified: trunk/Source/WebCore/style/StyleSharingResolver.h (198583 => 198584)
--- trunk/Source/WebCore/style/StyleSharingResolver.h 2016-03-23 14:05:54 UTC (rev 198583)
+++ trunk/Source/WebCore/style/StyleSharingResolver.h 2016-03-23 14:16:17 UTC (rev 198584)
@@ -26,12 +26,15 @@
#ifndef StyleSharingResolver_h
#define StyleSharingResolver_h
+#include <wtf/HashMap.h>
+
namespace WebCore {
class Document;
class DocumentRuleSets;
class Element;
class Node;
+class RenderStyle;
class RuleSet;
class SelectorFilter;
class SpaceSplitString;
@@ -43,13 +46,13 @@
public:
SharingResolver(const Document&, const DocumentRuleSets&, const SelectorFilter&);
- const Element* resolve(const Element&) const;
+ RefPtr<RenderStyle> resolve(const Element&);
private:
struct Context;
StyledElement* findSibling(const Context&, Node*, unsigned& count) const;
- Node* locateCousinList(Element* parent, unsigned& visitedNodeCount) const;
+ Node* locateCousinList(const Element* parent) const;
bool canShareStyleWithElement(const Context&, const StyledElement& candidateElement) const;
bool styleSharingCandidateMatchesRuleSet(const StyledElement&, const RuleSet*) const;
bool sharingCandidateHasIdenticalStyleAffectingAttributes(const Context&, const StyledElement& sharingCandidate) const;
@@ -58,6 +61,8 @@
const Document& m_document;
const DocumentRuleSets& m_ruleSets;
const SelectorFilter& m_selectorFilter;
+
+ HashMap<const Element*, const Element*> m_elementsSharingStyle;
};
}
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (198583 => 198584)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-03-23 14:05:54 UTC (rev 198583)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-03-23 14:16:17 UTC (rev 198584)
@@ -186,8 +186,8 @@
}
}
- if (auto* sharingElement = scope().sharingResolver.resolve(element))
- return *sharingElement->renderStyle();
+ if (auto style = scope().sharingResolver.resolve(element))
+ return *style;
auto elementStyle = scope().styleResolver.styleForElement(element, &inheritedStyle, MatchAllRules, nullptr, &scope().selectorFilter);
@@ -697,11 +697,6 @@
if (RenderElement* renderer = current.renderer()) {
if (localChange != NoChange || pseudoStyleCacheIsInvalid(renderer, newStyle.get()) || (parent().change == Force && renderer->requiresForcedStyleRecalcPropagation()) || current.styleChangeType() == SyntheticStyleChange)
renderer->setAnimatableStyle(*newStyle, current.styleChangeType() == SyntheticStyleChange ? StyleDifferenceRecompositeLayer : StyleDifferenceEqual);
- else if (current.needsStyleRecalc()) {
- // Although no change occurred, we use the new style so that the cousin style sharing code won't get
- // fooled into believing this style is the same.
- renderer->setStyleInternal(*newStyle);
- }
}
// If "rem" units are used anywhere in the document, and if the document element's font size changes, then force font updating
