Bug 474743 Patch D followup: Switch nsSMILTimedElement::mTimeDependents to be a hash table instead of an array. r=birtles sr=roc

content/smil/nsSMILTimedElement.cpp

@@ -126,6 +126,7 @@ nsSMILTimedElement::nsSMILTimedElement()
   mSimpleDur.SetIndefinite();
   mMin.SetMillis(0L);
   mMax.SetIndefinite();
+  mTimeDependents.Init();
 }
 
 void
@@ -840,23 +841,11 @@ nsSMILTimedElement::UnsetFillMode()
 void
 nsSMILTimedElement::AddDependent(nsSMILTimeValueSpec& aDependent)
 {
-  // Insert the dependent so that the array remains sorted.
-  // We could use InsertElementSorted but we want to ensure that we don't end up
-  // with duplicate entries.
-  PRUint32 index;
-  PRBool found = mTimeDependents.GreatestIndexLtEq(&aDependent,
-      nsDefaultComparator<nsSMILTimeValueSpec*, nsSMILTimeValueSpec*>(),
-      &index);
-
   // There's probably no harm in attempting to register a dependent
-  // nsSMILTimeValueSpec twice (as long as we don't add it to the array twice)
+  // nsSMILTimeValueSpec twice, but we're not expecting it to happen.
-  // but we're not expecting it to happen.
+  NS_ABORT_IF_FALSE(!mTimeDependents.GetEntry(&aDependent),
-  NS_ABORT_IF_FALSE(!found,
       "nsSMILTimeValueSpec is already registered as a dependency");
-  if (found)
+  mTimeDependents.PutEntry(&aDependent);
-    return;
-
-  mTimeDependents.InsertElementAt(index, &aDependent);
 
   if (mCurrentInterval.IsSet()) {
     // Not necessary to call SyncPauseTime here as we're dealing with
@@ -866,9 +855,9 @@ nsSMILTimedElement::AddDependent(nsSMILTimeValueSpec& aDependent)
 }
 
 void
-nsSMILTimedElement::RemoveDependent(const nsSMILTimeValueSpec& aDependent)
+nsSMILTimedElement::RemoveDependent(nsSMILTimeValueSpec& aDependent)
 {
-  mTimeDependents.RemoveElementSorted(&aDependent);
+  mTimeDependents.RemoveEntry(&aDependent);
 }
 
 PRBool
@@ -1544,13 +1533,8 @@ nsSMILTimedElement::NotifyNewInterval()
     container->SyncPauseTime();
   }
 
-  PRUint32 count = mTimeDependents.Length();
+  NotifyTimeDependentsParams params = { &mCurrentInterval, container };
-  for (PRUint32 i = 0; i < count; ++i) {
+  mTimeDependents.EnumerateEntries(NotifyNewIntervalCallback, &params);
-    nsSMILTimeValueSpec* spec = mTimeDependents[i];
-    NS_ABORT_IF_FALSE(spec,
-        "null nsSMILTimeValueSpec in list of time dependents");
-    spec->HandleNewInterval(mCurrentInterval, container);
-  }
 }
 
 void
@@ -1565,25 +1549,14 @@ nsSMILTimedElement::NotifyChangedInterval()
     container->SyncPauseTime();
   }
 
-  PRUint32 count = mTimeDependents.Length();
+  NotifyTimeDependentsParams params = { &mCurrentInterval, container };
-  for (PRUint32 i = 0; i < count; ++i) {
+  mTimeDependents.EnumerateEntries(NotifyChangedIntervalCallback, &params);
-    nsSMILTimeValueSpec* spec = mTimeDependents[i];
-    NS_ABORT_IF_FALSE(spec,
-        "null nsSMILTimeValueSpec in list of time dependents");
-    spec->HandleChangedInterval(mCurrentInterval, container);
-  }
 }
 
 void
 nsSMILTimedElement::NotifyDeletedInterval()
 {
-  PRUint32 count = mTimeDependents.Length();
+  mTimeDependents.EnumerateEntries(NotifyDeletedIntervalCallback, nsnull);
-  for (PRUint32 i = 0; i < count; ++i) {
-    nsSMILTimeValueSpec* spec = mTimeDependents[i];
-    NS_ABORT_IF_FALSE(spec,
-        "null nsSMILTimeValueSpec in list of time dependents");
-    spec->HandleDeletedInterval();
-  }
 }
 
 const nsSMILInstanceTime*
@@ -1606,3 +1579,59 @@ nsSMILTimedElement::GetEffectiveBeginInstance() const
     return nsnull;
   }
 }
+
+//----------------------------------------------------------------------
+// Hashtable callback functions
+
+/* static */ PR_CALLBACK PLDHashOperator
+nsSMILTimedElement::NotifyNewIntervalCallback(TimeValueSpecPtrKey* aKey,
+                                              void* aData)
+{
+  NotifyTimeDependentsParams* params =
+    static_cast<NotifyTimeDependentsParams*>(aData);
+  SanityCheckTimeDependentCallbackArgs(aKey, params, PR_TRUE);
+
+  nsSMILTimeValueSpec* spec = aKey->GetKey();
+  spec->HandleNewInterval(*params->mCurrentInterval, params->mTimeContainer);
+  return PL_DHASH_NEXT;
+}
+
+/* static */ PR_CALLBACK PLDHashOperator
+nsSMILTimedElement::NotifyChangedIntervalCallback(TimeValueSpecPtrKey* aKey,
+                                                  void* aData)
+{
+  NotifyTimeDependentsParams* params =
+    static_cast<NotifyTimeDependentsParams*>(aData);
+  SanityCheckTimeDependentCallbackArgs(aKey, params, PR_TRUE);
+
+  nsSMILTimeValueSpec* spec = aKey->GetKey();
+  spec->HandleChangedInterval(*params->mCurrentInterval,
+                              params->mTimeContainer);
+  return PL_DHASH_NEXT;
+}
+
+/* static */ PR_CALLBACK PLDHashOperator
+nsSMILTimedElement::NotifyDeletedIntervalCallback(TimeValueSpecPtrKey* aKey,
+                                                  void* /* unused */)
+{
+  SanityCheckTimeDependentCallbackArgs(aKey, nsnull, PR_FALSE);
+
+  nsSMILTimeValueSpec* spec = aKey->GetKey();
+  spec->HandleDeletedInterval();
+  return PL_DHASH_NEXT;
+}
+
+/* static */ void
+nsSMILTimedElement::SanityCheckTimeDependentCallbackArgs(
+    TimeValueSpecPtrKey* aKey,
+    NotifyTimeDependentsParams* aParams,
+    PRBool aExpectingParams)
+{
+  NS_ABORT_IF_FALSE(aKey, "Null hash key for time container hash table");
+  NS_ABORT_IF_FALSE(aKey->GetKey(),
+                    "null nsSMILTimeValueSpec in set of time dependents");
+  if (aExpectingParams) {
+    NS_ABORT_IF_FALSE(aParams, "null data ptr while enumerating hashtable");
+    NS_ABORT_IF_FALSE(aParams->mCurrentInterval, "null current-interval ptr");
+  }
+}

content/smil/nsSMILTimedElement.h

@@ -45,6 +45,8 @@
 #include "nsSMILRepeatCount.h"
 #include "nsSMILTypes.h"
 #include "nsTArray.h"
+#include "nsTHashtable.h"
+#include "nsHashKeys.h"
 #include "nsAutoPtr.h"
 #include "nsAttrValue.h"
 
@@ -271,7 +273,7 @@ public:
    *
    * @param aDependent  The nsSMILTimeValueSpec object to unregister.
    */
-  void RemoveDependent(const nsSMILTimeValueSpec& aDependent);
+  void RemoveDependent(nsSMILTimeValueSpec& aDependent);
 
   /**
    * Determines if this timed element is dependent on the given timed element's
@@ -312,6 +314,8 @@ protected:
   // Typedefs
   typedef nsTArray<nsAutoPtr<nsSMILTimeValueSpec> > TimeValueSpecList;
   typedef nsTArray<nsRefPtr<nsSMILInstanceTime> >   InstanceTimeList;
+  typedef nsPtrHashKey<nsSMILTimeValueSpec> TimeValueSpecPtrKey;
+  typedef nsTHashtable<TimeValueSpecPtrKey> TimeValueSpecHashSet;
 
   // Helper classes
   class InstanceTimeComparator {
@@ -322,6 +326,11 @@ protected:
                       const nsSMILInstanceTime* aElem2) const;
   };
 
+  struct NotifyTimeDependentsParams {
+    nsSMILInterval*      mCurrentInterval;
+    nsSMILTimeContainer* mTimeContainer;
+  };
+
   //
   // Implementation helpers
   //
@@ -404,6 +413,17 @@ protected:
   void              NotifyDeletedInterval();
   const nsSMILInstanceTime* GetEffectiveBeginInstance() const;
 
+  // Hashtable callback methods
+  PR_STATIC_CALLBACK(PLDHashOperator) NotifyNewIntervalCallback(
+      TimeValueSpecPtrKey* aKey, void* aData);
+  PR_STATIC_CALLBACK(PLDHashOperator) NotifyChangedIntervalCallback(
+      TimeValueSpecPtrKey* aKey, void* aData);
+  PR_STATIC_CALLBACK(PLDHashOperator) NotifyDeletedIntervalCallback(
+      TimeValueSpecPtrKey* aKey, void* /* unused */);
+  static inline void SanityCheckTimeDependentCallbackArgs(
+      TimeValueSpecPtrKey* aKey, NotifyTimeDependentsParams* aParams,
+      PRBool aExpectingParams);
+
   //
   // Members
   //
@@ -456,16 +476,12 @@ protected:
   nsSMILMilestone                 mPrevRegisteredMilestone;
   static const nsSMILMilestone    sMaxMilestone;
 
-  // List of dependent time value specs to be notified when creating, updating,
+  // Set of dependent time value specs to be notified when creating, updating,
   // or deleting the current interval.
   //
   // [weak] The nsSMILTimeValueSpec objects register themselves and unregister
   // on destruction. Likewise, we notify them when we are destroyed.
-  //
+  TimeValueSpecHashSet mTimeDependents;
-  // To avoid worst-case O(n^2) performance when many time dependents want to
-  // unregister, we keep these arrays sorted so we have worst case O(n*logn) for
-  // add and remove.
-  nsTArray<nsSMILTimeValueSpec*>  mTimeDependents;
 
   /**
    * The state of the element in its life-cycle. These states are based on the