Bug 1498316 - Limit the number of nsFontMetrics entries cached by each device context, to avoid excessive growth of this cache in examples such as animated font variations or sizes. r=lsalzman
☠☠ backed out by f30aa69dd3b1 ☠ ☠
authorJonathan Kew <jkew@mozilla.com>
Fri, 12 Oct 2018 09:25:27 +0100
changeset 499310 57e52347a69b17244922a9a2b60be27db3279689
parent 499309 a6fe44fb6369e1230322a6a1d9d03064e568b7a5
child 499311 f30aa69dd3b1290de0b8808f44f7c69bdcb6861b
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1498316
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1498316 - Limit the number of nsFontMetrics entries cached by each device context, to avoid excessive growth of this cache in examples such as animated font variations or sizes. r=lsalzman
gfx/src/nsDeviceContext.cpp
--- a/gfx/src/nsDeviceContext.cpp
+++ b/gfx/src/nsDeviceContext.cpp
@@ -60,21 +60,31 @@ public:
 
     void FontMetricsDeleted(const nsFontMetrics* aFontMetrics);
     void Compact();
     void Flush();
 
     void UpdateUserFonts(gfxUserFontSet* aUserFontSet);
 
 protected:
+    // If the array of cached entries is about to exceed this threshold,
+    // we'll discard the oldest ones so as to keep the size reasonable.
+    // In practice, the great majority of cache hits are among the last
+    // few entries; keeping thousands of older entries becomes counter-
+    // productive because it can then take too long to scan the cache.
+    static const int32_t kMaxCacheEntries = 128;
+
     ~nsFontCache() {}
 
     nsDeviceContext*          mContext; // owner
     RefPtr<nsAtom>         mLocaleLanguage;
-    nsTArray<nsFontMetrics*>  mFontMetrics;
+
+    // We don't allow this array to grow beyond kMaxCacheEntries,
+    // so use an autoarray to avoid separate allocation.
+    AutoTArray<nsFontMetrics*,kMaxCacheEntries> mFontMetrics;
 };
 
 NS_IMPL_ISUPPORTS(nsFontCache, nsIObserver)
 
 // The Init and Destroy methods are necessary because it's not
 // safe to call AddObserver from a constructor or RemoveObserver
 // from a destructor.  That should be fixed.
 void
@@ -114,18 +124,17 @@ already_AddRefed<nsFontMetrics>
 nsFontCache::GetMetricsFor(const nsFont& aFont,
                            const nsFontMetrics::Params& aParams)
 {
     nsAtom* language = aParams.language ? aParams.language
                                          : mLocaleLanguage.get();
 
     // First check our cache
     // start from the end, which is where we put the most-recent-used element
-
-    int32_t n = mFontMetrics.Length() - 1;
+    const int32_t n = mFontMetrics.Length() - 1;
     for (int32_t i = n; i >= 0; --i) {
         nsFontMetrics* fm = mFontMetrics[i];
         if (fm->Font().Equals(aFont) &&
             fm->GetUserFontSet() == aParams.userFontSet &&
             fm->Language() == language &&
             fm->Orientation() == aParams.orientation) {
             if (i != n) {
                 // promote it to the end of the cache
@@ -133,16 +142,21 @@ nsFontCache::GetMetricsFor(const nsFont&
                 mFontMetrics.AppendElement(fm);
             }
             fm->GetThebesFontGroup()->UpdateUserFonts();
             return do_AddRef(fm);
         }
     }
 
     // It's not in the cache. Get font metrics and then cache them.
+    // If the cache has reached its size limit, drop the older half of the
+    // entries.
+    if (n >= kMaxCacheEntries - 1) {
+        mFontMetrics.RemoveElementsAt(0, kMaxCacheEntries / 2);
+    }
 
     nsFontMetrics::Params params = aParams;
     params.language = language;
     RefPtr<nsFontMetrics> fm = new nsFontMetrics(aFont, params, mContext);
     // the mFontMetrics list has the "head" at the end, because append
     // is cheaper than insert
     mFontMetrics.AppendElement(do_AddRef(fm).take());
     return fm.forget();