Use random() instead of begin() to limit cache sizes
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Oct 2018 17:07:49 +0000 (17:07 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Oct 2018 17:07:49 +0000 (17:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190957

Reviewed by Chris Dumez.

Source/WebCore:

We currently use cache.remove(cache.begin()) pattern to limit sized of various caches.
This is a bad pattern for tables that never rehash (because they have fixed maximum size) as most of the
keys get permanently stuck in the table.

* css/CSSValuePool.cpp:
(WebCore::CSSValuePool::createColorValue):
(WebCore::CSSValuePool::createFontFamilyValue):
(WebCore::CSSValuePool::createFontFaceValue):
* dom/InlineStyleSheetOwner.cpp:
(WebCore::InlineStyleSheetOwner::createSheet):
* dom/SelectorQuery.cpp:
* platform/graphics/FontCascade.cpp:
(WebCore::retrieveOrAddCachedFonts):
* platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::shouldAutoActivateFontIfNeeded):
* platform/mac/PublicSuffixMac.mm:
(WebCore::topPrivatelyControlledDomain):

Source/WebKit:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::didCollectPrewarmInformation):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237463 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/CSSValuePool.cpp
Source/WebCore/dom/InlineStyleSheetOwner.cpp
Source/WebCore/dom/SelectorQuery.cpp
Source/WebCore/platform/graphics/FontCascade.cpp
Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp
Source/WebCore/platform/mac/PublicSuffixMac.mm
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebProcessPool.cpp

index 581c3df..5f7f87d 100644 (file)
@@ -1,3 +1,28 @@
+2018-10-26  Antti Koivisto  <antti@apple.com>
+
+        Use random() instead of begin() to limit cache sizes
+        https://bugs.webkit.org/show_bug.cgi?id=190957
+
+        Reviewed by Chris Dumez.
+
+        We currently use cache.remove(cache.begin()) pattern to limit sized of various caches.
+        This is a bad pattern for tables that never rehash (because they have fixed maximum size) as most of the
+        keys get permanently stuck in the table.
+
+        * css/CSSValuePool.cpp:
+        (WebCore::CSSValuePool::createColorValue):
+        (WebCore::CSSValuePool::createFontFamilyValue):
+        (WebCore::CSSValuePool::createFontFaceValue):
+        * dom/InlineStyleSheetOwner.cpp:
+        (WebCore::InlineStyleSheetOwner::createSheet):
+        * dom/SelectorQuery.cpp:
+        * platform/graphics/FontCascade.cpp:
+        (WebCore::retrieveOrAddCachedFonts):
+        * platform/graphics/cocoa/FontCacheCoreText.cpp:
+        (WebCore::shouldAutoActivateFontIfNeeded):
+        * platform/mac/PublicSuffixMac.mm:
+        (WebCore::topPrivatelyControlledDomain):
+
 2018-10-26  Jer Noble  <jer.noble@apple.com>
 
         Adopt -setOverrideRouteSharingPolicy:routingContextUID: SPI
index 235f76f..fbd6b16 100644 (file)
@@ -87,7 +87,7 @@ Ref<CSSPrimitiveValue> CSSValuePool::createColorValue(const Color& color)
     // FIXME: Use TinyLRUCache instead?
     const int maximumColorCacheSize = 512;
     if (m_colorValueCache.size() >= maximumColorCacheSize)
-        m_colorValueCache.remove(m_colorValueCache.begin());
+        m_colorValueCache.remove(m_colorValueCache.random());
 
     return *m_colorValueCache.ensure(color, [&color] {
         return CSSPrimitiveValue::create(color);
@@ -123,7 +123,7 @@ Ref<CSSPrimitiveValue> CSSValuePool::createFontFamilyValue(const String& familyN
     // FIXME: Use TinyLRUCache instead?
     const int maximumFontFamilyCacheSize = 128;
     if (m_fontFamilyValueCache.size() >= maximumFontFamilyCacheSize)
-        m_fontFamilyValueCache.remove(m_fontFamilyValueCache.begin());
+        m_fontFamilyValueCache.remove(m_fontFamilyValueCache.random());
 
     bool isFromSystemID = fromSystemFontID == FromSystemFontID::Yes;
     return *m_fontFamilyValueCache.ensure({ familyName, isFromSystemID }, [&familyName, isFromSystemID] {
@@ -137,7 +137,7 @@ RefPtr<CSSValueList> CSSValuePool::createFontFaceValue(const AtomicString& strin
     // FIXME: Use TinyLRUCache instead?
     const int maximumFontFaceCacheSize = 128;
     if (m_fontFaceValueCache.size() >= maximumFontFaceCacheSize)
-        m_fontFaceValueCache.remove(m_fontFaceValueCache.begin());
+        m_fontFaceValueCache.remove(m_fontFaceValueCache.random());
 
     return m_fontFaceValueCache.ensure(string, [&string] () -> RefPtr<CSSValueList> {
         auto result = CSSParser::parseSingleValue(CSSPropertyFontFamily, string);
index 8eec26a..ca531f0 100644 (file)
@@ -218,8 +218,9 @@ void InlineStyleSheetOwner::createSheet(Element& element, const String& text)
         // Prevent pathological growth.
         const size_t maximumInlineStyleSheetCacheSize = 50;
         if (inlineStyleSheetCache().size() > maximumInlineStyleSheetCacheSize) {
-            inlineStyleSheetCache().begin()->value->removedFromMemoryCache();
-            inlineStyleSheetCache().remove(inlineStyleSheetCache().begin());
+            auto toRemove = inlineStyleSheetCache().random();
+            toRemove->value->removedFromMemoryCache();
+            inlineStyleSheetCache().remove(toRemove);
         }
     }
 }
index 79e862d..acec1c2 100644 (file)
@@ -631,7 +631,7 @@ ExceptionOr<SelectorQuery&> SelectorQueryCache::add(const String& selectors, Doc
 
     const int maximumSelectorQueryCacheSize = 256;
     if (m_entries.size() == maximumSelectorQueryCacheSize)
-        m_entries.remove(m_entries.begin());
+        m_entries.remove(m_entries.random());
 
     return *m_entries.add(selectors, std::make_unique<SelectorQuery>(WTFMove(selectorList))).iterator->value;
 }
index 06620cf..7438213 100644 (file)
@@ -265,7 +265,7 @@ static Ref<FontCascadeFonts> retrieveOrAddCachedFonts(const FontCascadeDescripti
         pruneUnreferencedEntriesFromFontCascadeCache();
     // Prevent pathological growth.
     if (fontCascadeCache().size() > maximumEntries)
-        fontCascadeCache().remove(fontCascadeCache().begin());
+        fontCascadeCache().remove(fontCascadeCache().random());
     return glyphs;
 }
 
index c9ef35d..30244cf 100644 (file)
@@ -1238,7 +1238,7 @@ static bool shouldAutoActivateFontIfNeeded(const AtomicString& family)
     static const unsigned maxCacheSize = 128;
     ASSERT(knownFamilies.get().size() <= maxCacheSize);
     if (knownFamilies.get().size() == maxCacheSize)
-        knownFamilies.get().remove(knownFamilies.get().begin());
+        knownFamilies.get().remove(knownFamilies.get().random());
 
     // Only attempt to auto-activate fonts once for performance reasons.
     return knownFamilies.get().add(family).isNewEntry;
index e2edb87..cd1cc73 100644 (file)
@@ -59,7 +59,7 @@ String topPrivatelyControlledDomain(const String& domain)
 
     constexpr auto maximumSizeToPreventUnlimitedGrowth = 128;
     if (cache.get().size() == maximumSizeToPreventUnlimitedGrowth)
-        cache.get().clear();
+        cache.get().remove(cache.get().random());
 
     return cache.get().ensure(isolatedDomain, [&isolatedDomain] {
         const auto lowercaseDomain = isolatedDomain.convertToASCIILowercase();
index f4768e0..bcd8222 100644 (file)
@@ -1,3 +1,13 @@
+2018-10-26  Antti Koivisto  <antti@apple.com>
+
+        Use random() instead of begin() to limit cache sizes
+        https://bugs.webkit.org/show_bug.cgi?id=190957
+
+        Reviewed by Chris Dumez.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::didCollectPrewarmInformation):
+
 2018-10-26  Chris Dumez  <cdumez@apple.com>
 
         Modernize / Simplify IPC::Connection::sendOutgoingMessage()
index fa56a2c..9995c05 100644 (file)
@@ -2282,7 +2282,7 @@ void WebProcessPool::didCollectPrewarmInformation(const String& registrableDomai
 {
     static const size_t maximumSizeToPreventUnlimitedGrowth = 100;
     if (m_prewarmInformationPerRegistrableDomain.size() == maximumSizeToPreventUnlimitedGrowth)
-        m_prewarmInformationPerRegistrableDomain.remove(m_prewarmInformationPerRegistrableDomain.begin());
+        m_prewarmInformationPerRegistrableDomain.remove(m_prewarmInformationPerRegistrableDomain.random());
 
     auto& value = m_prewarmInformationPerRegistrableDomain.ensure(registrableDomain, [] {
         return std::make_unique<WebCore::PrewarmInformation>();