Modified: trunk/Source/WebCore/ChangeLog (101523 => 101524)
--- trunk/Source/WebCore/ChangeLog 2011-11-30 17:16:45 UTC (rev 101523)
+++ trunk/Source/WebCore/ChangeLog 2011-11-30 17:25:17 UTC (rev 101524)
@@ -1,3 +1,52 @@
+2011-11-30 Antti Koivisto <an...@apple.com>
+
+ Reuse cached style fully if the parent inherited styles are equal
+ https://bugs.webkit.org/show_bug.cgi?id=73421
+
+ Reviewed by Oliver Hunt.
+
+ The matched declaration cache currently restores the non-inherted properties from the cache
+ entry but still applies all inherited properties normally. In case the current parent
+ inherited style is equivalent to the cache entry's, also the inherited style can be reused
+ and no properties need to be applied. This is faster and saves memory (by sharing the
+ style substructures better).
+
+ The new optimized code path has a pretty good hit rate, >50% of all cases on many pages.
+
+ Loading the HTML5 spec this reduces style memory consumption by ~20% (5MB, ~2.5% of total) and
+ speeds up style applying by ~25% for ~0.4s (2-3%) gain in the spec loading benchmark.
+
+ * css/CSSStyleSelector.cpp:
+ (WebCore::CSSStyleSelector::applyDeclaration):
+ (WebCore::CSSStyleSelector::applyDeclarations):
+
+ Remove the code that dynamically disables inherited only applying. We now don't allow
+ styles with explicitly inherited properties to be cached in the first place.
+
+ (WebCore::CSSStyleSelector::findFromMatchedDeclarationCache):
+
+ Return the full cache item.
+
+ (WebCore::CSSStyleSelector::addToMatchedDeclarationCache):
+
+ Also the parent style is now needed for the check for full sharing.
+
+ (WebCore::isCacheableInMatchedDeclarationCache):
+
+ Don't allow styles with explicitly inherited properties to be cached at all.
+
+ (WebCore::CSSStyleSelector::applyMatchedDeclarations):
+
+ If the parent inherited styles are equal reuse the cache entry fully and return without
+ doing anything else.
+
+ * css/CSSStyleSelector.h:
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::inheritedDataShared):
+ * rendering/style/RenderStyle.h:
+
+ Add fast check for equal inherited properties.
+
2011-11-30 Renata Hodovan <r...@webkit.org>
CG buildfix after r101517.
Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (101523 => 101524)
--- trunk/Source/WebCore/css/CSSStyleSelector.cpp 2011-11-30 17:16:45 UTC (rev 101523)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp 2011-11-30 17:25:17 UTC (rev 101524)
@@ -2119,7 +2119,7 @@
}
template <bool applyFirst>
-void CSSStyleSelector::applyDeclaration(CSSMutableStyleDeclaration* styleDeclaration, bool isImportant, bool& inheritedOnly)
+void CSSStyleSelector::applyDeclaration(CSSMutableStyleDeclaration* styleDeclaration, bool isImportant, bool inheritedOnly)
{
CSSMutableStyleDeclaration::const_iterator end = styleDeclaration->end();
for (CSSMutableStyleDeclaration::const_iterator it = styleDeclaration->begin(); it != end; ++it) {
@@ -2127,12 +2127,11 @@
if (isImportant != current.isImportant())
continue;
if (inheritedOnly && !current.isInherited()) {
- if (!current.value()->isInheritedValue())
- continue;
// If the property value is explicitly inherited, we need to apply further non-inherited properties
- // as they might override the value inherited here. This is really per-property but that is
- // probably not worth optimizing for.
- inheritedOnly = false;
+ // as they might override the value inherited here. For this reason we don't allow declarations with
+ // explicitly inherited properties to be cached.
+ ASSERT(!current.value()->isInheritedValue());
+ continue;
}
int property = current.id();
if (applyFirst) {
@@ -2156,7 +2155,7 @@
}
template <bool applyFirst>
-void CSSStyleSelector::applyDeclarations(bool isImportant, int startIndex, int endIndex, bool& inheritedOnly)
+void CSSStyleSelector::applyDeclarations(bool isImportant, int startIndex, int endIndex, bool inheritedOnly)
{
if (startIndex == -1)
return;
@@ -2210,7 +2209,7 @@
return !(a == b);
}
-const RenderStyle* CSSStyleSelector::findFromMatchedDeclarationCache(unsigned hash, const MatchResult& matchResult)
+const CSSStyleSelector::MatchedStyleDeclarationCacheItem* CSSStyleSelector::findFromMatchedDeclarationCache(unsigned hash, const MatchResult& matchResult)
{
ASSERT(hash);
@@ -2229,18 +2228,19 @@
}
if (cacheItem.matchResult != matchResult)
return 0;
- return cacheItem.renderStyle.get();
+ return &cacheItem;
}
-void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, unsigned hash, const MatchResult& matchResult)
+void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, const RenderStyle* parentStyle, unsigned hash, const MatchResult& matchResult)
{
ASSERT(hash);
MatchedStyleDeclarationCacheItem cacheItem;
cacheItem.matchedStyleDeclarations.append(m_matchedDecls);
cacheItem.matchResult = matchResult;
// Note that we don't cache the original RenderStyle instance. It may be further modified.
- // The RenderStyle in the cache is really just a holder for the non-inherited substructures and never used as-is.
+ // The RenderStyle in the cache is really just a holder for the substructures and never used as-is.
cacheItem.renderStyle = RenderStyle::clone(style);
+ cacheItem.parentRenderStyle = RenderStyle::clone(parentStyle);
m_matchStyleDeclarationCache.add(hash, cacheItem);
}
@@ -2252,6 +2252,9 @@
return false;
if (style->zoom() != RenderStyle::initialZoom())
return false;
+ // The cache assumes static knowledge about which properties are inherited.
+ if (parentStyle->hasExplicitlyInheritedProperties())
+ return false;
return true;
}
@@ -2259,12 +2262,21 @@
{
unsigned cacheHash = matchResult.isCacheable ? computeDeclarationHash(m_matchedDecls.data(), m_matchedDecls.size()) : 0;
bool applyInheritedOnly = false;
- const RenderStyle* cachedStyle = 0;
- if (cacheHash && (cachedStyle = findFromMatchedDeclarationCache(cacheHash, matchResult))) {
+ const MatchedStyleDeclarationCacheItem* cacheItem = 0;
+ if (cacheHash && (cacheItem = findFromMatchedDeclarationCache(cacheHash, matchResult))) {
// We can build up the style by copying non-inherited properties from an earlier style object built using the same exact
// style declarations. We then only need to apply the inherited properties, if any, as their values can depend on the
// element context. This is fast and saves memory by reusing the style data structures.
- m_style->copyNonInheritedFrom(cachedStyle);
+ m_style->copyNonInheritedFrom(cacheItem->renderStyle.get());
+ if (m_parentStyle->inheritedDataShared(cacheItem->parentRenderStyle.get())) {
+ EInsideLink linkStatus = m_style->insideLink();
+ // If the cache item parent style has identical inherited properties to the current parent style then the
+ // resulting style will be identical too. We copy the inherited properties over from the cache and are done.
+ m_style->inheritFrom(cacheItem->renderStyle.get());
+ // Unfortunately the link status is treated like an inherited property. We need to explicitly restore it.
+ m_style->setInsideLink(linkStatus);
+ return;
+ }
applyInheritedOnly = true;
}
// Now we have all of the matched rules in the appropriate order. Walk the rules and apply
@@ -2277,7 +2289,7 @@
applyDeclarations<true>(true, matchResult.firstUserRule, matchResult.lastUserRule, applyInheritedOnly);
applyDeclarations<true>(true, matchResult.firstUARule, matchResult.lastUARule, applyInheritedOnly);
- if (cachedStyle && cachedStyle->effectiveZoom() != m_style->effectiveZoom()) {
+ if (cacheItem && cacheItem->renderStyle->effectiveZoom() != m_style->effectiveZoom()) {
m_fontDirty = true;
applyInheritedOnly = false;
}
@@ -2290,7 +2302,7 @@
applyProperty(CSSPropertyLineHeight, m_lineHeightValue);
// Many properties depend on the font. If it changes we just apply all properties.
- if (cachedStyle && cachedStyle->fontDescription() != m_style->fontDescription())
+ if (cacheItem && cacheItem->renderStyle->fontDescription() != m_style->fontDescription())
applyInheritedOnly = false;
// Now do the normal priority UA properties.
@@ -2313,11 +2325,11 @@
ASSERT(!m_fontDirty);
- if (cachedStyle || !cacheHash)
+ if (cacheItem || !cacheHash)
return;
if (!isCacheableInMatchedDeclarationCache(m_style.get(), m_parentStyle))
return;
- addToMatchedDeclarationCache(m_style.get(), cacheHash, matchResult);
+ addToMatchedDeclarationCache(m_style.get(), m_parentStyle, cacheHash, matchResult);
}
void CSSStyleSelector::matchPageRules(RuleSet* rules, bool isLeftPage, bool isFirstPage, const String& pageName)
Modified: trunk/Source/WebCore/css/CSSStyleSelector.h (101523 => 101524)
--- trunk/Source/WebCore/css/CSSStyleSelector.h 2011-11-30 17:16:45 UTC (rev 101523)
+++ trunk/Source/WebCore/css/CSSStyleSelector.h 2011-11-30 17:25:17 UTC (rev 101524)
@@ -265,9 +265,9 @@
void applyMatchedDeclarations(const MatchResult&);
template <bool firstPass>
- void applyDeclarations(bool important, int startIndex, int endIndex, bool& inheritedOnly);
+ void applyDeclarations(bool important, int startIndex, int endIndex, bool inheritedOnly);
template <bool firstPass>
- void applyDeclaration(CSSMutableStyleDeclaration*, bool isImportant, bool& inheritedOnly);
+ void applyDeclaration(CSSMutableStyleDeclaration*, bool isImportant, bool inheritedOnly);
void matchPageRules(RuleSet*, bool isLeftPage, bool isFirstPage, const String& pageName);
void matchPageRulesForList(const Vector<RuleData>*, bool isLeftPage, bool isFirstPage, const String& pageName);
@@ -349,20 +349,21 @@
unsigned linkMatchType;
};
static unsigned computeDeclarationHash(MatchedStyleDeclaration*, unsigned size);
- const RenderStyle* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);
- void addToMatchedDeclarationCache(const RenderStyle*, unsigned hash, const MatchResult&);
+ struct MatchedStyleDeclarationCacheItem {
+ Vector<MatchedStyleDeclaration> matchedStyleDeclarations;
+ MatchResult matchResult;
+ RefPtr<RenderStyle> renderStyle;
+ RefPtr<RenderStyle> parentRenderStyle;
+ };
+ const MatchedStyleDeclarationCacheItem* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);
+ void addToMatchedDeclarationCache(const RenderStyle*, const RenderStyle* parentStyle, unsigned hash, const MatchResult&);
// We collect the set of decls that match in |m_matchedDecls|. We then walk the
// set of matched decls four times, once for those properties that others depend on (like font-size),
// and then a second time for all the remaining properties. We then do the same two passes
// for any !important rules.
Vector<MatchedStyleDeclaration, 64> m_matchedDecls;
-
- struct MatchedStyleDeclarationCacheItem {
- Vector<MatchedStyleDeclaration> matchedStyleDeclarations;
- MatchResult matchResult;
- RefPtr<RenderStyle> renderStyle;
- };
+
typedef HashMap<unsigned, MatchedStyleDeclarationCacheItem> MatchedStyleDeclarationCache;
MatchedStyleDeclarationCache m_matchStyleDeclarationCache;
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (101523 => 101524)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2011-11-30 17:16:45 UTC (rev 101523)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2011-11-30 17:25:17 UTC (rev 101524)
@@ -321,6 +321,17 @@
|| rareInheritedData != other->rareInheritedData;
}
+bool RenderStyle::inheritedDataShared(const RenderStyle* other) const
+{
+ // This is a fast check that only looks if the data structures are shared.
+ return inherited_flags == other->inherited_flags
+ && inherited.get() == other->inherited.get()
+#if ENABLE(SVG)
+ && m_svgStyle.get() == other->m_svgStyle.get()
+#endif
+ && rareInheritedData.get() == other->rareInheritedData.get();
+}
+
static bool positionedObjectMoved(const LengthBox& a, const LengthBox& b)
{
// If any unit types are different, then we can't guarantee
