Bug 941868 - Part 3: don't evict built-in bitmaps, either. r=mcomella, a=lsblakk
authorRichard Newman <rnewman@mozilla.com>
Tue, 26 Nov 2013 19:48:30 -0800
changeset 167696 3cf413884fdd02e332516044c7c59dcacc3a7780
parent 167695 dbe0e1ca219a0e84b06c52958d1c562f07611b35
child 167697 8e41eadc884ff9f4f005c072a588f5f3d9b76f8a
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella, lsblakk
bugs941868
milestone27.0
Bug 941868 - Part 3: don't evict built-in bitmaps, either. r=mcomella, a=lsblakk * * * Fixes for Aurora.
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();
         }
     }
 }