Bug 1471025: Part 1 - Store preference access counts in a separate hashtable. r=njn

Once the majority of preferences are stored in a read-only shared map, it
won't be possible to modify the access counts in their entries. Which means we
need a separate map for the access counts.

Fortunately, this code is only active in debug builds, so it shouldn't affect
release users. And the net impact on memory usage of this patchset will still
be decidedly downward.

MozReview-Commit-ID: EuLXlMQJP1M

--HG--
extra : intermediate-source : c490838c8329f6b0c21fa57ef078c44bf7a9ba8d
extra : absorb_source : 37a0f7a5fecba6c28bd6ce30644543c6d60ac823
extra : source : 4a8fbb472c91f13554cac3d0ea638cf9f368ff11
extra : histedit_source : 0e5a5f1cded97bcc959be15aa7194c017fd7efcc%2Cc0ae2011b9b40e6445ee366303f8ec4bb10cc87b

browser/base/content/test/performance/browser_preferences_usage.js

@@ -89,6 +89,9 @@ add_task(async function startup() {
       min: 45,
       max: 75,
     },
+    "network.loadinfo.skip_type_assertion": {
+      max: 650,
+    },
     "extensions.getAddons.cache.enabled": {
       min: 9,
       max: 55,
@@ -138,8 +141,11 @@ add_task(async function open_10_tabs() {
     "browser.startup.record": {
       max: 20,
     },
-    "dom.max_chrome_script_run_time": {
-      max: 20,
+    "browser.tabs.remote.logSwitchTiming": {
+      max: 25,
+    },
+    "network.loadinfo.skip_type_assertion": {
+      max: 70,
     },
     "toolkit.cosmeticAnimations.enabled": {
       min: 5,
@@ -170,6 +176,9 @@ add_task(async function navigate_around() {
       min: 100,
       max: 110,
     },
+    "network.loadinfo.skip_type_assertion": {
+      max: 110,
+    },
     "security.insecure_connection_icon.pbmode.enabled": {
       min: 20,
       max: 30,

modules/libpref/Preferences.cpp

@@ -967,10 +967,6 @@ private:
 class PrefEntry : public PLDHashEntryHdr
 {
 public:
-#ifdef DEBUG
-  // This field is before mPref to minimize sizeof(PrefEntry) on 64-bit.
-  uint32_t mAccessCount;
-#endif
   Pref* mPref; // Note: this is never null in a live entry.
 
   static bool MatchEntry(const PLDHashEntryHdr* aEntry, const void* aKey)
@@ -986,9 +982,6 @@ public:
     auto entry = static_cast<PrefEntry*>(aEntry);
     auto prefName = static_cast<const char*>(aKey);
 
-#ifdef DEBUG
-    entry->mAccessCount = 0;
-#endif
     entry->mPref = new Pref(prefName);
   }
 
@@ -1076,6 +1069,43 @@ static PLDHashTable* gHashTable;
 static CallbackNode* gFirstCallback = nullptr;
 static CallbackNode* gLastPriorityNode = nullptr;
 
+#ifdef DEBUG
+#define ACCESS_COUNTS
+#endif
+
+#ifdef ACCESS_COUNTS
+using AccessCountsHashTable = nsDataHashtable<nsCStringHashKey, uint32_t>;
+static AccessCountsHashTable* gAccessCounts;
+
+static void
+AddAccessCount(const nsACString& aPrefName)
+{
+  // FIXME: Servo reads preferences from background threads in unsafe ways (bug
+  // 1474789), and triggers assertions here if we try to add usage count entries
+  // from background threads.
+  if (NS_IsMainThread()) {
+    uint32_t& count = gAccessCounts->GetOrInsert(aPrefName);
+    count++;
+  }
+}
+
+static void
+AddAccessCount(const char* aPrefName)
+{
+  AddAccessCount(nsDependentCString(aPrefName));
+}
+#else
+static void MOZ_MAYBE_UNUSED
+AddAccessCount(const nsACString& aPrefName)
+{
+}
+
+static void
+AddAccessCount(const char* aPrefName)
+{
+}
+#endif
+
 // These are only used during the call to NotifyCallbacks().
 static bool gCallbacksInProgress = false;
 static bool gShouldCleanupDeadNodes = false;
@@ -1147,9 +1177,7 @@ pref_HashTableLookup(const char* aPrefName)
     return nullptr;
   }
 
-#ifdef DEBUG
-  entry->mAccessCount += 1;
-#endif
+  AddAccessCount(aPrefName);
 
   return entry->mPref;
 }
@@ -3275,6 +3303,11 @@ Preferences::GetInstanceForService()
   gTelemetryLoadData =
     new nsDataHashtable<nsCStringHashKey, TelemetryLoadData>();
 
+#ifdef ACCESS_COUNTS
+  MOZ_ASSERT(!gAccessCounts);
+  gAccessCounts = new AccessCountsHashTable();
+#endif
+
   gCacheData = new nsTArray<nsAutoPtr<CacheData>>();
   gCacheDataDesc = "set by GetInstanceForService() (1)";
 
@@ -3408,6 +3441,10 @@ Preferences::~Preferences()
   delete gTelemetryLoadData;
   gTelemetryLoadData = nullptr;
 
+#ifdef ACCESS_COUNTS
+  delete gAccessCounts;
+#endif
+
   gPrefNameArena.Clear();
 }
 
@@ -3744,10 +3781,9 @@ Preferences::GetDefaultBranch(const char* aPrefRoot, nsIPrefBranch** aRetVal)
 NS_IMETHODIMP
 Preferences::ReadStats(nsIPrefStatsCallback* aCallback)
 {
-#ifdef DEBUG
-  for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
-    PrefEntry* entry = static_cast<PrefEntry*>(iter.Get());
-    aCallback->Visit(entry->mPref->Name(), entry->mAccessCount);
+#ifdef ACCESS_COUNTS
+  for (auto iter = gAccessCounts->Iter(); !iter.Done(); iter.Next()) {
+    aCallback->Visit(iter.Key(), iter.Data());
   }
 
   return NS_OK;
@@ -3759,10 +3795,8 @@ Preferences::ReadStats(nsIPrefStatsCallback* aCallback)
 NS_IMETHODIMP
 Preferences::ResetStats()
 {
-#ifdef DEBUG
-  for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
-    static_cast<PrefEntry*>(iter.Get())->mAccessCount = 0;
-  }
+#ifdef ACCESS_COUNTS
+  gAccessCounts->Clear();
   return NS_OK;
 #else
   return NS_ERROR_NOT_IMPLEMENTED;

modules/libpref/nsIPrefService.idl

@@ -15,7 +15,7 @@ interface nsIFile;
 [function, scriptable, uuid(c3f0cedc-e244-4316-b33a-80306a1c35a1)]
 interface nsIPrefStatsCallback : nsISupports
 {
-  void visit(in string prefName, in unsigned long accessCount);
+  void visit(in ACString prefName, in unsigned long accessCount);
 };
 
 /**