Modified: trunk/Source/WebCore/ChangeLog (104549 => 104550)
--- trunk/Source/WebCore/ChangeLog 2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/ChangeLog 2012-01-10 11:35:32 UTC (rev 104550)
@@ -1,3 +1,29 @@
+2012-01-10 Andreas Kling <[email protected]>
+
+ Matched declaration cache should support mapped attribute declarations.
+ <http://webkit.org/b/75948>
+
+ Reviewed by Antti Koivisto.
+
+ Support caching of matches that include mapped attribute declarations.
+ To make this possible, let the cache hold references to the matched style declarations.
+ Otherwise, declarations are assumed to survive until ~CSSStyleSelector, which may not
+ be the case for CSSMappedAttributeDeclaration.
+
+ To avoid hoarding of stale CSSMappedAttributeDeclarations in the cache, do a sweep for
+ every 100 additions and garbage collect any entries containing a declaration that is
+ only referenced by the cache.
+
+ This increases cache hit rate by up to ~10% on the Alexa top sites.
+
+ * css/CSSStyleSelector.cpp:
+ (WebCore::CSSStyleSelector::CSSStyleSelector):
+ (WebCore::CSSStyleSelector::sweepMatchedDeclarationCache):
+ (WebCore::CSSStyleSelector::matchAllRules):
+ (WebCore::CSSStyleSelector::applyDeclarations):
+ (WebCore::CSSStyleSelector::addToMatchedDeclarationCache):
+ * css/CSSStyleSelector.h:
+
2012-01-10 Kenichi Ishibashi <[email protected]>
Complex path should be used when UVS exists in text run
Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (104549 => 104550)
--- trunk/Source/WebCore/css/CSSStyleSelector.cpp 2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp 2012-01-10 11:35:32 UTC (rev 104550)
@@ -330,6 +330,7 @@
bool strictParsing, bool matchAuthorAndUserStyles)
: m_hasUAAppearance(false)
, m_backgroundData(BackgroundFillLayer)
+ , m_matchedDeclarationCacheAdditionsSinceLastSweep(0)
, m_checker(document, strictParsing)
, m_parentStyle(0)
, m_rootElementStyle(0)
@@ -481,6 +482,27 @@
m_fontSelector->clearDocument();
}
+void CSSStyleSelector::sweepMatchedDeclarationCache()
+{
+ // Look for cache entries containing a style declaration with a single ref and remove them.
+ // This may happen when an element attribute mutation causes it to swap out its Attribute::decl()
+ // for another CSSMappedAttributeDeclaration, potentially leaving this cache with the last ref.
+ Vector<unsigned, 16> toRemove;
+ MatchedStyleDeclarationCache::iterator it = m_matchedStyleDeclarationCache.begin();
+ MatchedStyleDeclarationCache::iterator end = m_matchedStyleDeclarationCache.end();
+ for (; it != end; ++it) {
+ Vector<MatchedStyleDeclaration>& matchedStyleDeclarations = it->second.matchedStyleDeclarations;
+ for (size_t i = 0; i < matchedStyleDeclarations.size(); ++i) {
+ if (matchedStyleDeclarations[i].styleDeclaration->hasOneRef()) {
+ toRemove.append(it->first);
+ break;
+ }
+ }
+ }
+ for (size_t i = 0; i < toRemove.size(); ++i)
+ m_matchedStyleDeclarationCache.remove(toRemove[i]);
+}
+
CSSStyleSelector::Features::Features()
: usesFirstLineRules(false)
, usesBeforeAfterRules(false)
@@ -626,12 +648,6 @@
#endif
}
-CSSStyleSelector::MatchedStyleDeclaration::MatchedStyleDeclaration()
-{
- // Make sure all memory is zero initializes as we calculate hash over the bytes of this object.
- memset(this, 0, sizeof(*this));
-}
-
void CSSStyleSelector::addMatchedDeclaration(CSSMutableStyleDeclaration* styleDeclaration, unsigned linkMatchType)
{
m_matchedDecls.grow(m_matchedDecls.size() + 1);
@@ -820,7 +836,6 @@
if (result.firstAuthorRule == -1)
result.firstAuthorRule = result.lastAuthorRule;
addMatchedDeclaration(attr->decl());
- result.isCacheable = false;
}
}
}
@@ -2239,7 +2254,7 @@
if (m_style->insideLink() != NotInsideLink) {
for (int i = startIndex; i <= endIndex; ++i) {
- CSSMutableStyleDeclaration* styleDeclaration = m_matchedDecls[i].styleDeclaration;
+ CSSMutableStyleDeclaration* styleDeclaration = m_matchedDecls[i].styleDeclaration.get();
unsigned linkMatchType = m_matchedDecls[i].linkMatchType;
// FIXME: It would be nicer to pass these as arguments but that requires changes in many places.
m_applyPropertyToRegularStyle = linkMatchType & SelectorChecker::MatchLink;
@@ -2252,7 +2267,7 @@
return;
}
for (int i = startIndex; i <= endIndex; ++i)
- applyDeclaration<applyFirst>(m_matchedDecls[i].styleDeclaration, isImportant, inheritedOnly);
+ applyDeclaration<applyFirst>(m_matchedDecls[i].styleDeclaration.get(), isImportant, inheritedOnly);
}
unsigned CSSStyleSelector::computeDeclarationHash(MatchedStyleDeclaration* declarations, unsigned size)
@@ -2310,6 +2325,12 @@
void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, const RenderStyle* parentStyle, unsigned hash, const MatchResult& matchResult)
{
+ static unsigned matchedDeclarationCacheAdditionsBetweenSweeps = 100;
+ if (++m_matchedDeclarationCacheAdditionsSinceLastSweep >= matchedDeclarationCacheAdditionsBetweenSweeps) {
+ sweepMatchedDeclarationCache();
+ m_matchedDeclarationCacheAdditionsSinceLastSweep = 0;
+ }
+
ASSERT(hash);
MatchedStyleDeclarationCacheItem cacheItem;
cacheItem.matchedStyleDeclarations.append(m_matchedDecls);
Modified: trunk/Source/WebCore/css/CSSStyleSelector.h (104549 => 104550)
--- trunk/Source/WebCore/css/CSSStyleSelector.h 2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/css/CSSStyleSelector.h 2012-01-10 11:35:32 UTC (rev 104550)
@@ -346,9 +346,14 @@
void loadPendingImages();
struct MatchedStyleDeclaration {
- MatchedStyleDeclaration();
- CSSMutableStyleDeclaration* styleDeclaration;
- unsigned linkMatchType;
+ MatchedStyleDeclaration() : possiblyPaddedMember(0) { }
+
+ RefPtr<CSSMutableStyleDeclaration> styleDeclaration;
+ union {
+ unsigned linkMatchType;
+ // Used to make sure all memory is zero-initialized since we compute the hash over the bytes of this object.
+ void* possiblyPaddedMember;
+ };
};
static unsigned computeDeclarationHash(MatchedStyleDeclaration*, unsigned size);
struct MatchedStyleDeclarationCacheItem {
@@ -360,11 +365,16 @@
const MatchedStyleDeclarationCacheItem* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);
void addToMatchedDeclarationCache(const RenderStyle*, const RenderStyle* parentStyle, unsigned hash, const MatchResult&);
+ // Every N additions to the matched declaration cache trigger a sweep where entries holding
+ // the last reference to a style declaration are garbage collected.
+ void sweepMatchedDeclarationCache();
+
// 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;
+ unsigned m_matchedDeclarationCacheAdditionsSinceLastSweep;
typedef HashMap<unsigned, MatchedStyleDeclarationCacheItem> MatchedStyleDeclarationCache;
MatchedStyleDeclarationCache m_matchedStyleDeclarationCache;