Bug 561076 - Use a hashtable to speed up nsContentUtils::UnregisterPrefCallback. r=bzbarsky a=blocking2.0

--HG--
extra : rebase_source : 753885fe23666d392cea378b2d911a754df7ef58

content/base/public/nsContentUtils.h

@@ -69,6 +69,7 @@
 #include "nsIPrefBranch2.h"
 #include "mozilla/AutoRestore.h"
 #include "nsINode.h"
+#include "nsHashtable.h"
 
 #include "jsapi.h"
 
@@ -110,6 +111,7 @@ class nsIScriptContext;
 class nsIRunnable;
 class nsIInterfaceRequestor;
 template<class E> class nsCOMArray;
+template<class K, class V> class nsRefPtrHashtable;
 struct JSRuntime;
 class nsIUGenCategory;
 class nsIWidget;
@@ -119,6 +121,7 @@ class nsPIDOMEventTarget;
 class nsIPresShell;
 class nsIXPConnectJSObjectHolder;
 class nsPrefOldCallback;
+class nsPrefObserverHashKey;
 #ifdef MOZ_XTF
 class nsIXTFService;
 #endif
@@ -1710,7 +1713,8 @@ private:
 
   static nsIPrefBranch2 *sPrefBranch;
   // For old compatibility of RegisterPrefCallback
-  static nsCOMArray<nsPrefOldCallback> *sPrefCallbackList;
+  static nsRefPtrHashtable<nsPrefObserverHashKey, nsPrefOldCallback>
+    *sPrefCallbackTable;
 
   static bool sImgLoaderInitialized;
   static void InitImgLoader();

content/base/src/nsContentUtils.cpp

@@ -262,7 +262,8 @@ PRBool nsContentUtils::sIsHandlingKeyBoardEvent = PR_FALSE;
 
 PRBool nsContentUtils::sInitialized = PR_FALSE;
 
-nsCOMArray<nsPrefOldCallback> *nsContentUtils::sPrefCallbackList = nsnull;
+nsRefPtrHashtable<nsPrefObserverHashKey, nsPrefOldCallback>
+  *nsContentUtils::sPrefCallbackTable = nsnull;
 
 static PLDHashTable sEventListenerManagersHash;
 
@@ -313,39 +314,95 @@ class nsSameOriginChecker : public nsIChannelEventSink,
   NS_DECL_NSIINTERFACEREQUESTOR
 };
 
+class nsPrefObserverHashKey : public PLDHashEntryHdr {
+public:
+  typedef nsPrefObserverHashKey* KeyType;
+  typedef const nsPrefObserverHashKey* KeyTypePointer;
+
+  static const nsPrefObserverHashKey* KeyToPointer(nsPrefObserverHashKey *aKey)
+  {
+    return aKey;
+  }
+
+  static PLDHashNumber HashKey(const nsPrefObserverHashKey *aKey)
+  {
+    PRUint32 strHash = nsCRT::HashCode(aKey->mPref.BeginReading(),
+                                       aKey->mPref.Length());
+    return PR_ROTATE_LEFT32(strHash, 4) ^
+           NS_PTR_TO_UINT32(aKey->mCallback);
+  }
+
+  nsPrefObserverHashKey(const char *aPref, PrefChangedFunc aCallback) :
+    mPref(aPref), mCallback(aCallback) { }
+
+  nsPrefObserverHashKey(const nsPrefObserverHashKey *aOther) :
+    mPref(aOther->mPref), mCallback(aOther->mCallback)
+  { }
+
+  PRBool KeyEquals(const nsPrefObserverHashKey *aOther) const
+  {
+    return mCallback == aOther->mCallback &&
+           mPref.Equals(aOther->mPref);
+  }
+
+  nsPrefObserverHashKey *GetKey() const
+  {
+    return const_cast<nsPrefObserverHashKey*>(this);
+  }
+
+  enum { ALLOW_MEMMOVE = PR_TRUE };
+
+public:
+  nsCString mPref;
+  PrefChangedFunc mCallback;
+};
+
 // For nsContentUtils::RegisterPrefCallback/UnregisterPrefCallback
-class nsPrefOldCallback : public nsIObserver
+class nsPrefOldCallback : public nsIObserver,
+                          public nsPrefObserverHashKey
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
 public:
-  nsPrefOldCallback(const char *aPref, PrefChangedFunc aCallback, void *aClosure) : mPref(aPref), mCallback(aCallback), mClosure(aClosure) {
+  nsPrefOldCallback(const char *aPref, PrefChangedFunc aCallback)
+    : nsPrefObserverHashKey(aPref, aCallback) { }
+
+  ~nsPrefOldCallback() {
+    nsContentUtils::GetPrefBranch()->RemoveObserver(mPref.get(), this);
   }
 
-  PRBool IsEqual(const char *aPref, PrefChangedFunc aCallback, void *aClosure) {
-    return aCallback == mCallback &&
-           aClosure == mClosure &&
-           mPref.Equals(aPref);
+  void AppendClosure(void *aClosure) {
+    mClosures.AppendElement(aClosure);
+  }
+
+  void RemoveClosure(void *aClosure) {
+    mClosures.RemoveElement(aClosure);
+  }
+
+  PRBool HasNoClosures() {
+    return mClosures.Length() == 0;
   }
 
 public:
-  nsCString       mPref;
-  PrefChangedFunc mCallback;
-  void            *mClosure;
+  nsTObserverArray<void *> mClosures;
 };
 
 NS_IMPL_ISUPPORTS1(nsPrefOldCallback, nsIObserver)
 
 NS_IMETHODIMP
-nsPrefOldCallback::Observe(nsISupports   *aSubject,
-                         const char      *aTopic,
-                         const PRUnichar *aData)
+nsPrefOldCallback::Observe(nsISupports     *aSubject,
+                           const char      *aTopic,
+                           const PRUnichar *aData)
 {
   NS_ASSERTION(!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID),
                "invalid topic");
-  mCallback(NS_LossyConvertUTF16toASCII(aData).get(), mClosure);
+  NS_LossyConvertUTF16toASCII data(aData);
+  nsTObserverArray<void *>::ForwardIterator iter(mClosures);
+  while (iter.HasMore()) {
+    mCallback(data.get(), iter.GetNext());
+  }
 
   return NS_OK;
 }
@@ -1052,16 +1109,9 @@ nsContentUtils::Shutdown()
     NS_IF_RELEASE(sStringBundles[i]);
 
   // Clean up c-style's observer 
-  if (sPrefCallbackList) {
-    while (sPrefCallbackList->Count() > 0) {
-      nsRefPtr<nsPrefOldCallback> callback = (*sPrefCallbackList)[0];
-      NS_ABORT_IF_FALSE(callback, "Invalid c-style callback is appended");
-      if (sPrefBranch)
-        sPrefBranch->RemoveObserver(callback->mPref.get(), callback);
-      sPrefCallbackList->RemoveObject(callback);
-    }
-    delete sPrefCallbackList;
-    sPrefCallbackList = nsnull;
+  if (sPrefCallbackTable) {
+    delete sPrefCallbackTable;
+    sPrefCallbackTable = nsnull;
   }
 
   delete sPrefCacheData;
@@ -2620,8 +2670,8 @@ nsContentUtils::GetStringPref(const char *aPref)
   return result;
 }
 
-// RegisterPrefCallback/UnregisterPrefCallback are backward compatiblity for
-// c-style observer.
+// RegisterPrefCallback/UnregisterPrefCallback are for backward compatiblity
+// with c-style observers.
 
 // static
 void
@@ -2630,20 +2680,24 @@ nsContentUtils::RegisterPrefCallback(const char *aPref,
                                      void * aClosure)
 {
   if (sPrefBranch) {
-    if (!sPrefCallbackList) {
-      sPrefCallbackList = new nsCOMArray<nsPrefOldCallback> ();
-      if (!sPrefCallbackList)
-        return;
+    if (!sPrefCallbackTable) {
+      sPrefCallbackTable = 
+        new nsRefPtrHashtable<nsPrefObserverHashKey, nsPrefOldCallback>();
+      sPrefCallbackTable->Init();
     }
 
-    nsPrefOldCallback *callback = new nsPrefOldCallback(aPref, aCallback, aClosure);
+    nsPrefObserverHashKey hashKey(aPref, aCallback);
+    nsRefPtr<nsPrefOldCallback> callback;
+    sPrefCallbackTable->Get(&hashKey, getter_AddRefs(callback));
     if (callback) {
-      if (NS_SUCCEEDED(sPrefBranch->AddObserver(aPref, callback, PR_FALSE))) {
-        sPrefCallbackList->AppendObject(callback);
-        return;
-      }
-      // error to get/add nsIPrefBranch2.  Destroy callback information
-      delete callback;
+      callback->AppendClosure(aClosure);
+      return;
+    }
+
+    callback = new nsPrefOldCallback(aPref, aCallback);
+    callback->AppendClosure(aClosure);
+    if (NS_SUCCEEDED(sPrefBranch->AddObserver(aPref, callback, PR_FALSE))) {
+      sPrefCallbackTable->Put(callback, callback);
     }
   }
 }
@@ -2655,16 +2709,19 @@ nsContentUtils::UnregisterPrefCallback(const char *aPref,
                                        void * aClosure)
 {
   if (sPrefBranch) {
-    if (!sPrefCallbackList)
+    if (!sPrefCallbackTable) {
       return;
+    }
 
-    int i;
-    for (i = 0; i < sPrefCallbackList->Count(); i++) {
-      nsRefPtr<nsPrefOldCallback> callback = (*sPrefCallbackList)[i];
-      if (callback && callback->IsEqual(aPref, aCallback, aClosure)) {
-        sPrefBranch->RemoveObserver(aPref, callback);
-        sPrefCallbackList->RemoveObject(callback);
-        return;
+    nsPrefObserverHashKey hashKey(aPref, aCallback);
+    nsRefPtr<nsPrefOldCallback> callback;
+    sPrefCallbackTable->Get(&hashKey, getter_AddRefs(callback));
+
+    if (callback) {
+      callback->RemoveClosure(aClosure);
+      if (callback->HasNoClosures()) {
+        // Delete the callback since its list of closures is empty.
+        sPrefCallbackTable->Remove(callback);
       }
     }
   }

modules/libpref/src/prefapi.cpp

@@ -852,9 +852,9 @@ PREF_UnregisterCallback(const char *pref_node,
 
     while (node != NULL)
     {
-        if ( strcmp(node->domain, pref_node) == 0 &&
-             node->func == callback &&
-             node->data == instance_data)
+        if ( node->func == callback &&
+             node->data == instance_data &&
+             strcmp(node->domain, pref_node) == 0)
         {
             if (gCallbacksInProgress)
             {