Bug 1277064 - Use ConcurrentHashMap for profile cache; r=snorp
authorJim Chen <nchen@mozilla.com>
Fri, 03 Jun 2016 12:01:17 -0400
changeset 339470 e8d8e9ee556559be8abf1944099617970d6a07ff
parent 339469 465fe1502bf49a376d9237bd1438d8a729adb770
child 339471 91a825772c31d0fde133f44d5739e784613835b6
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1277064
milestone49.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 1277064 - Use ConcurrentHashMap for profile cache; r=snorp Use ConcurrentHashMap for the profile cache so that accessing the cache won't cause inadvertent deadlocks.
mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ -34,19 +34,19 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.nio.charset.Charset;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public final class GeckoProfile {
     private static final String LOGTAG = "GeckoProfile";
 
     // The path in the profile to the file containing the client ID.
     private static final String CLIENT_ID_FILE_PATH = "datareporting/state.json";
@@ -74,17 +74,19 @@ public final class GeckoProfile {
 
     // Session store
     private static final String SESSION_FILE = "sessionstore.js";
     private static final String SESSION_FILE_BACKUP = "sessionstore.bak";
     private static final long MAX_BACKUP_FILE_AGE = 1000 * 3600 * 24; // 24 hours
 
     private boolean mOldSessionDataProcessed = false;
 
-    private static final HashMap<String, GeckoProfile> sProfileCache = new HashMap<String, GeckoProfile>();
+    private static final ConcurrentHashMap<String, GeckoProfile> sProfileCache =
+            new ConcurrentHashMap<String, GeckoProfile>(
+                    /* capacity */ 4, /* load factor */ 0.75f, /* concurrency */ 2);
     private static String sDefaultProfileName;
 
     private final String mName;
     private final File mMozillaDir;
     private final Context mApplicationContext;
 
     private final BrowserDB mDB;
 
@@ -157,17 +159,17 @@ public final class GeckoProfile {
         }
     }
 
     public static GeckoProfile get(Context context) {
         return get(context, null, null, null);
     }
 
     public static GeckoProfile get(Context context, String profileName) {
-        synchronized (sProfileCache) {
+        if (profileName != null) {
             GeckoProfile profile = sProfileCache.get(profileName);
             if (profile != null)
                 return profile;
         }
         return get(context, profileName, (File)null);
     }
 
     @RobocopTarget
@@ -237,61 +239,66 @@ public final class GeckoProfile {
         } else if (profileName == null) {
             // If only profile dir was passed in, use custom (anonymous) profile.
             profileName = CUSTOM_PROFILE;
 
         } else if (AppConstants.DEBUG_BUILD) {
             Log.v(LOGTAG, "Fetching profile: '" + profileName + "', '" + profileDir + "'");
         }
 
+        // We require the profile dir to exist if specified, so create it here if needed.
+        final boolean init = profileDir != null && profileDir.mkdirs();
+
         // Actually try to look up the profile.
-        synchronized (sProfileCache) {
-            // We require the profile dir to exist if specified, so create it here if needed.
-            final boolean init = profileDir != null && profileDir.mkdirs();
+        GeckoProfile profile = sProfileCache.get(profileName);
+        GeckoProfile newProfile = null;
+
+        if (profile == null) {
+            try {
+                newProfile = new GeckoProfile(context, profileName, profileDir, dbFactory);
+            } catch (NoMozillaDirectoryException e) {
+                // We're unable to do anything sane here.
+                throw new RuntimeException(e);
+            }
 
-            GeckoProfile profile = sProfileCache.get(profileName);
-            if (profile == null) {
-                try {
-                    profile = new GeckoProfile(context, profileName, profileDir, dbFactory);
-                } catch (NoMozillaDirectoryException e) {
-                    // We're unable to do anything sane here.
-                    throw new RuntimeException(e);
-                }
-                sProfileCache.put(profileName, profile);
+            profile = sProfileCache.putIfAbsent(profileName, newProfile);
+        }
+
+        if (profile == null) {
+            profile = newProfile;
 
-            } else if (profileDir != null) {
-                // We have an existing profile but was given an alternate directory.
-                boolean consistent = false;
-                try {
-                    consistent = profile.getDir().getCanonicalPath().equals(
-                            profileDir.getCanonicalPath());
-                } catch (final IOException e) {
+        } else if (profileDir != null) {
+            // We have an existing profile but was given an alternate directory.
+            boolean consistent = false;
+            try {
+                consistent = profile.mProfileDir != null &&
+                        profile.mProfileDir.getCanonicalPath().equals(profileDir.getCanonicalPath());
+            } catch (final IOException e) {
+            }
+
+            if (!consistent) {
+                if (!sAcceptDirectoryChanges || !profileDir.isDirectory()) {
+                    throw new IllegalStateException(
+                            "Refusing to reuse profile with a different directory.");
                 }
 
-                if (!consistent) {
-                    if (!sAcceptDirectoryChanges || !profileDir.isDirectory()) {
-                        throw new IllegalStateException(
-                                "Refusing to reuse profile with a different directory.");
-                    }
+                if (AppConstants.RELEASE_BUILD) {
+                    Log.e(LOGTAG, "Release build trying to switch out profile dir. " +
+                                  "This is an error, but let's do what we can.");
+                }
+                profile.setDir(profileDir);
+            }
+        }
 
-                    if (AppConstants.RELEASE_BUILD) {
-                        Log.e(LOGTAG, "Release build trying to switch out profile dir. " +
-                                      "This is an error, but let's do what we can.");
-                    }
-                    profile.setDir(profileDir);
-                }
-            }
+        if (init) {
+            // Initialize the profile directory if we had to create it.
+            profile.enqueueInitialization(profileDir);
+        }
 
-            if (init) {
-                // Initialize the profile directory if we had to create it.
-                profile.enqueueInitialization(profileDir);
-            }
-
-            return profile;
-        }
+        return profile;
     }
 
     // Currently unused outside of testing.
     @RobocopTarget
     public static boolean removeProfile(final Context context, final GeckoProfile profile) {
         final boolean success = profile.remove();
 
         if (success) {