Bug 941868 - Part 3: don't evict built-in bitmaps, either. r=mcomella
authorRichard Newman <rnewman@mozilla.com>
Tue, 26 Nov 2013 19:48:30 -0800
changeset 157728 cffd41d1d7e8226f4ff014108e6cf107b62becff
parent 157727 b5a3121b05c11057ba3e145789a786ec9d43a563
child 157729 a164ef0013e736d508732fee4f6ad68275e461b3
push id25721
push usercbook@mozilla.com
push dateWed, 27 Nov 2013 10:02:03 +0000
treeherdermozilla-central@6ecf0c4dfcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella
bugs941868
milestone28.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 941868 - Part 3: don't evict built-in bitmaps, either. r=mcomella
mobile/android/base/favicons/Favicons.java
mobile/android/base/favicons/cache/FaviconCache.java
--- a/mobile/android/base/favicons/Favicons.java
+++ b/mobile/android/base/favicons/Favicons.java
@@ -284,18 +284,18 @@ public class Favicons {
 
         return taskId;
     }
 
     public static void putFaviconInMemCache(String pageUrl, Bitmap image) {
         sFaviconsCache.putSingleFavicon(pageUrl, image);
     }
 
-    public static void putFaviconsInMemCache(String pageUrl, Iterator<Bitmap> images) {
-        sFaviconsCache.putFavicons(pageUrl, images);
+    public static void putFaviconsInMemCache(String pageUrl, Iterator<Bitmap> images, boolean permanently) {
+        sFaviconsCache.putFavicons(pageUrl, images, permanently);
     }
 
     public static void clearMemCache() {
         sFaviconsCache.evictAll();
         sPageURLMappings.evictAll();
     }
 
     public static void putFaviconInFailedCache(String faviconURL) {
@@ -372,17 +372,17 @@ public class Favicons {
             sPageURLMappings.putWithoutEviction(url, BUILT_IN_FAVICON_URL);
         }
 
         // Load and cache the built-in favicon in each of its sizes.
         // TODO: don't open the zip twice!
         ArrayList<Bitmap> toInsert = new ArrayList<Bitmap>(2);
         toInsert.add(loadBrandingBitmap(context, "favicon64.png"));
         toInsert.add(loadBrandingBitmap(context, "favicon32.png"));
-        putFaviconsInMemCache(BUILT_IN_FAVICON_URL, toInsert.iterator());
+        putFaviconsInMemCache(BUILT_IN_FAVICON_URL, toInsert.iterator(), true);
     }
 
     /**
      * Compute a string like:
      * "jar:jar:file:///data/app/org.mozilla.firefox-1.apk!/assets/omni.ja!/chrome/chrome/content/branding/favicon64.png"
      */
     private static String getBrandingBitmapPath(Context context, String name) {
         final String apkPath = context.getPackageResourcePath();
--- a/mobile/android/base/favicons/cache/FaviconCache.java
+++ b/mobile/android/base/favicons/cache/FaviconCache.java
@@ -18,16 +18,19 @@ import java.util.concurrent.atomic.Atomi
  * Implements a Least-Recently-Used cache for Favicons, keyed by Favicon URL.
  *
  * When a favicon at a particular URL is decoded, it will yield one or more bitmaps.
  * While in memory, these bitmaps are stored in a list, sorted in ascending order of size, in a
  * FaviconsForURL object.
  * The collection of FaviconsForURL objects currently in the cache is stored in mBackingMap, keyed
  * by favicon URL.
  *
+ * A second map exists for permanent cache entries -- ones that are never expired. These entries
+ * are assumed to be disjoint from those in the normal cache, and this map is checked first.
+ *
  * FaviconsForURL provides a method for obtaining the smallest icon larger than a given size - the
  * most appropriate icon for a particular size.
  * It also distinguishes between "primary" favicons (Ones that have merely been extracted from a
  * file downloaded from the website) and "secondary" favicons (Ones that have been computed locally
  * as resized versions of primary favicons.).
  *
  * FaviconsForURL is also responsible for storing URL-specific, as opposed to favicon-specific,
  * information. For the purposes of this cache, the simplifying assumption that the dominant colour
@@ -101,28 +104,31 @@ public class FaviconCache {
     public static final long FAILURE_RETRY_MILLISECONDS = 1000 * 60 * 20;
 
     // Map relating Favicon URLs with objects representing decoded favicons.
     // Since favicons may be container formats holding multiple icons, the underlying type holds a
     // sorted list of bitmap payloads in ascending order of size. The underlying type may be queried
     // for the least larger payload currently present.
     private final ConcurrentHashMap<String, FaviconsForURL> mBackingMap = new ConcurrentHashMap<String, FaviconsForURL>();
 
+    // And the same, but never evicted.
+    private final ConcurrentHashMap<String, FaviconsForURL> mPermanentBackingMap = new ConcurrentHashMap<String, FaviconsForURL>();
+
     // A linked list used to implement a queue, defining the LRU properties of the cache. Elements
     // contained within the various FaviconsForURL objects are held here, the least recently used
     // of which at the end of the list. When space needs to be reclaimed, the appropriate bitmap is
     // culled.
     private final LinkedList<FaviconCacheElement> mOrdering = new LinkedList<FaviconCacheElement>();
 
     // The above structures, if used correctly, enable this cache to exhibit LRU semantics across all
     // favicon payloads in the system, as well as enabling the dynamic selection from the cache of
     // the primary bitmap most suited to the requested size (in cases where multiple primary bitmaps
     // are provided by the underlying file format).
 
-    // Current size, in bytes, of the bitmap data present in the cache.
+    // Current size, in bytes, of the bitmap data present in the LRU cache.
     private final AtomicInteger mCurrentSize = new AtomicInteger(0);
 
     // The maximum quantity, in bytes, of bitmap data which may be stored in the cache.
     private final int mMaxSizeBytes;
 
     // Tracks the number of ongoing read operations. Enables the first one in to lock writers out and
     // the last one out to let them in.
     private final AtomicInteger mOngoingReads = new AtomicInteger(0);
@@ -211,16 +217,18 @@ public class FaviconCache {
 
         startRead();
 
         boolean isExpired = false;
         boolean isAborting = false;
 
         try {
             // If we don't have it in the cache, it certainly isn't a known failure.
+            // Non-evictable favicons are never failed, so we don't need to
+            // check mPermanentBackingMap.
             if (!mBackingMap.containsKey(faviconURL)) {
                 return false;
             }
 
             FaviconsForURL container = mBackingMap.get(faviconURL);
 
             // If the has failed flag is not set, it's certainly not a known failure.
             if (!container.mHasFailed) {
@@ -299,28 +307,34 @@ public class FaviconCache {
         if (faviconURL == null) {
             Log.e(LOGTAG, "You passed a null faviconURL to getFaviconForDimensions. Don't.");
             return null;
         }
 
         boolean doingWrites = false;
         boolean shouldComputeColour = false;
         boolean isAborting = false;
+        boolean wasPermanent = false;
+        FaviconsForURL container;
         final Bitmap newBitmap;
-        final FaviconsForURL container;
 
         startRead();
 
         try {
-            if (!mBackingMap.containsKey(faviconURL)) {
-                return null;
+            container = mPermanentBackingMap.get(faviconURL);
+            if (container == null) {
+                container = mBackingMap.get(faviconURL);
+                if (container == null) {
+                    // We don't have it!
+                    return null;
+                }
+            } else {
+                wasPermanent = true;
             }
 
-            container = mBackingMap.get(faviconURL);
-
             FaviconCacheElement cacheElement;
 
             int cacheElementIndex = container.getNextHighestIndex(targetSize);
 
             // cacheElementIndex now holds either the index of the next least largest bitmap from
             // targetSize, or -1 if targetSize > all bitmaps.
             if (cacheElementIndex != -1) {
                 // If cacheElementIndex is not the sentinel value, then it is a valid index into mFavicons.
@@ -407,19 +421,20 @@ public class FaviconCache {
             }
 
             // While the image might not actually BE that size, we set the size field to the target
             // because this is the best image you can get for a request of that size using the Favicon
             // information provided by this website.
             // This way, subsequent requests hit straight away.
             FaviconCacheElement newElement = container.addSecondary(newBitmap, targetSize);
 
-            setMostRecentlyUsed(newElement);
-
-            mCurrentSize.addAndGet(newElement.sizeOf());
+            if (!wasPermanent) {
+                setMostRecentlyUsed(newElement);
+                mCurrentSize.addAndGet(newElement.sizeOf());
+            }
         } finally {
             finishWrite();
         }
 
         return newBitmap;
     }
 
     /**
@@ -427,24 +442,28 @@ public class FaviconCache {
      *
      * @param key The URL of the Favicon for which a dominant colour is desired.
      * @return The cached dominant colour, or null if none is cached.
      */
     public int getDominantColor(String key) {
         startRead();
 
         try {
-            if (!mBackingMap.containsKey(key)) {
+            FaviconsForURL element = mPermanentBackingMap.get(key);
+            if (element == null) {
+                element = mBackingMap.get(key);
+            }
+
+            if (element == null) {
                 Log.w(LOGTAG, "Cannot compute dominant color of non-cached favicon. Cache fullness " +
                               mCurrentSize.get() + '/' + mMaxSizeBytes);
                 finishRead();
                 return 0xFFFFFF;
             }
 
-            FaviconsForURL element = mBackingMap.get(key);
 
             return element.ensureDominantColor();
         } finally {
             finishRead();
         }
     }
 
     /**
@@ -538,18 +557,19 @@ public class FaviconCache {
         cullIfRequired();
     }
 
     /**
      * Set the collection of primary favicons for the given URL to the provided collection of bitmaps.
      *
      * @param faviconURL The URL from which the favicons originate.
      * @param favicons A List of favicons decoded from this URL.
+     * @param permanently If true, the added favicons are never subject to eviction.
      */
-    public void putFavicons(String faviconURL, Iterator<Bitmap> favicons) {
+    public void putFavicons(String faviconURL, Iterator<Bitmap> favicons, boolean permanently) {
         // We don't know how many icons we'll have - let's just take a guess.
         FaviconsForURL toInsert = new FaviconsForURL(5 * NUM_FAVICON_SIZES);
         int sizeGained = 0;
 
         while (favicons.hasNext()) {
             Bitmap favicon = produceCacheableBitmap(favicons.next());
             if (favicon == null) {
                 continue;
@@ -562,38 +582,44 @@ public class FaviconCache {
         startRead();
 
         boolean abortingRead = false;
 
         // Not using setMostRecentlyUsed, because the elements are known to be new. This can be done
         // without taking the write lock, via the magic of the reordering semaphore.
         mReorderingSemaphore.acquireUninterruptibly();
         try {
-            for (FaviconCacheElement newElement : toInsert.mFavicons) {
-                mOrdering.offer(newElement);
+            if (!permanently) {
+                for (FaviconCacheElement newElement : toInsert.mFavicons) {
+                    mOrdering.offer(newElement);
+                }
             }
         } catch (Exception e) {
             abortingRead = true;
             mReorderingSemaphore.release();
             finishRead();
 
             Log.e(LOGTAG, "Favicon cache exception!", e);
             return;
         } finally {
             if (!abortingRead) {
                 mReorderingSemaphore.release();
                 upgradeReadToWrite();
             }
         }
 
         try {
-            mCurrentSize.addAndGet(sizeGained);
+            if (permanently) {
+                mPermanentBackingMap.put(faviconURL, toInsert);
+            } else {
+                mCurrentSize.addAndGet(sizeGained);
 
-            // Update the value in the LruCache...
-            recordRemoved(mBackingMap.put(faviconURL, toInsert));
+                // Update the value in the LruCache...
+                recordRemoved(mBackingMap.put(faviconURL, toInsert));
+            }
         } finally {
             finishWrite();
         }
 
         cullIfRequired();
     }
 
     /**
@@ -626,17 +652,19 @@ public class FaviconCache {
     }
 
     /**
      * Purge all elements from the FaviconCache. Handy if you want to reclaim some memory.
      */
     public void evictAll() {
         startWrite();
 
+        // Note that we neither clear, nor track the size of, the permanent map.
         try {
             mCurrentSize.set(0);
             mBackingMap.clear();
             mOrdering.clear();
+
         } finally {
             finishWrite();
         }
     }
 }