Bug 1615564, clearing timeouts should be O(1) r=farre

Differential Revision: https://phabricator.services.mozilla.com/D74566

dom/base/Timeout.h

@@ -15,6 +15,7 @@
 #include "nsGlobalWindowInner.h"
 #include "nsCycleCollectionParticipant.h"
 #include "GeckoProfiler.h"
+#include "nsDataHashtable.h"
 
 class nsIEventTarget;
 class nsIPrincipal;
@@ -28,7 +29,7 @@ namespace dom {
  * timeout.  Holds a strong reference to an nsITimeoutHandler, which
  * abstracts the language specific cruft.
  */
-class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
+class Timeout final : protected LinkedListElement<RefPtr<Timeout>> {
  public:
   Timeout();
 
@@ -40,6 +41,45 @@ class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
     eIdleCallbackTimeout,
   };
 
+  struct TimeoutIdAndReason {
+    uint32_t mId;
+    Reason mReason;
+  };
+
+  class TimeoutHashKey : public PLDHashEntryHdr {
+   public:
+    typedef const TimeoutIdAndReason& KeyType;
+    typedef const TimeoutIdAndReason* KeyTypePointer;
+
+    explicit TimeoutHashKey(KeyTypePointer aKey) : mValue(*aKey) {}
+    TimeoutHashKey(TimeoutHashKey&& aOther)
+        : PLDHashEntryHdr(std::move(aOther)),
+          mValue(std::move(aOther.mValue)) {}
+    ~TimeoutHashKey() = default;
+
+    KeyType GetKey() const { return mValue; }
+    bool KeyEquals(KeyTypePointer aKey) const {
+      return aKey->mId == mValue.mId && aKey->mReason == mValue.mReason;
+    }
+
+    static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
+    static PLDHashNumber HashKey(KeyTypePointer aKey) {
+      return aKey->mId | (static_cast<uint8_t>(aKey->mReason) << 31);
+    }
+    enum { ALLOW_MEMMOVE = true };
+
+   private:
+    const TimeoutIdAndReason mValue;
+  };
+
+  class TimeoutSet : public nsDataHashtable<TimeoutHashKey, Timeout*> {
+   public:
+    NS_INLINE_DECL_REFCOUNTING(TimeoutSet);
+
+   private:
+    ~TimeoutSet() = default;
+  };
+
   void SetWhenOrTimeRemaining(const TimeStamp& aBaseTime,
                               const TimeDuration& aDelay);
 
@@ -51,6 +91,35 @@ class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
   // Can only be called when frozen.
   const TimeDuration& TimeRemaining() const;
 
+  void SetTimeoutContainer(TimeoutSet* aTimeouts) {
+    MOZ_ASSERT(mTimeoutId != 0);
+    TimeoutIdAndReason key = {mTimeoutId, mReason};
+    if (mTimeouts) {
+      mTimeouts->Remove(key);
+    }
+    mTimeouts = aTimeouts;
+    if (mTimeouts) {
+      mTimeouts->Put(key, this);
+    }
+  }
+
+  // Override some LinkedListElement methods so that remove()
+  // calls can call SetTimeoutContainer.
+  Timeout* getNext() { return LinkedListElement<RefPtr<Timeout>>::getNext(); }
+
+  void setNext(Timeout* aNext) {
+    return LinkedListElement<RefPtr<Timeout>>::setNext(aNext);
+  }
+
+  Timeout* getPrevious() {
+    return LinkedListElement<RefPtr<Timeout>>::getPrevious();
+  }
+
+  void remove() {
+    SetTimeoutContainer(nullptr);
+    LinkedListElement<RefPtr<Timeout>>::remove();
+  }
+
 #ifdef MOZ_GECKO_PROFILER
   UniqueProfilerBacktrace TakeProfilerBacktrace() { return std::move(mCause); }
 #endif
@@ -70,7 +139,7 @@ class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
   // (or is cancelled, etc)
   TimeStamp mSubmitTime;
 
-  ~Timeout() = default;
+  ~Timeout() { SetTimeoutContainer(nullptr); }
 
  public:
   // Public member variables in this section.  Please don't add to this list
@@ -84,6 +153,8 @@ class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
   // The language-specific information about the callback.
   RefPtr<TimeoutHandler> mScriptHandler;
 
+  RefPtr<TimeoutSet> mTimeouts;
+
   // Interval
   TimeDuration mInterval;
 
@@ -122,6 +193,10 @@ class Timeout final : public LinkedListElement<RefPtr<Timeout>> {
 
   // True if this is a repeating/interval timer
   bool mIsInterval;
+
+ protected:
+  friend class LinkedList<RefPtr<Timeout>>;
+  friend class LinkedListElement<RefPtr<Timeout>>;
 };
 
 }  // namespace dom

dom/base/TimeoutManager.cpp

@@ -521,9 +521,10 @@ nsresult TimeoutManager::SetTimeout(TimeoutHandler* aHandler, int32_t interval,
 
   Timeouts::SortBy sort(mWindow.IsFrozen() ? Timeouts::SortBy::TimeRemaining
                                            : Timeouts::SortBy::TimeWhen);
-  mTimeouts.Insert(timeout, sort);
 
   timeout->mTimeoutId = GetTimeoutId(aReason);
+  mTimeouts.Insert(timeout, sort);
+
   *aReturn = timeout->mTimeoutId;
 
   MOZ_LOG(
@@ -557,35 +558,31 @@ bool TimeoutManager::ClearTimeoutInternal(int32_t aTimerId,
   uint32_t timerId = (uint32_t)aTimerId;
   Timeouts& timeouts = aIsIdle ? mIdleTimeouts : mTimeouts;
   RefPtr<TimeoutExecutor>& executor = aIsIdle ? mIdleExecutor : mExecutor;
-  bool firstTimeout = true;
   bool deferredDeletion = false;
-  bool cleared = false;
-
-  timeouts.ForEachAbortable([&](Timeout* aTimeout) {
-    MOZ_LOG(gTimeoutLog, LogLevel::Debug,
-            ("Clear%s(TimeoutManager=%p, timeout=%p, aTimerId=%u, ID=%u)\n",
-             aTimeout->mIsInterval ? "Interval" : "Timeout", this, aTimeout,
-             timerId, aTimeout->mTimeoutId));
-
-    if (aTimeout->mTimeoutId == timerId && aTimeout->mReason == aReason) {
-      if (aTimeout->mRunning) {
-        /* We're running from inside the aTimeout. Mark this
-           aTimeout for deferred deletion by the code in
-           RunTimeout() */
-        aTimeout->mIsInterval = false;
-        deferredDeletion = true;
-      } else {
-        /* Delete the aTimeout from the pending aTimeout list */
-        aTimeout->remove();
-      }
-      cleared = true;
-      return true;  // abort!
-    }
-
-    firstTimeout = false;
 
+  Timeout* timeout = timeouts.GetTimeout(aTimerId, aReason);
+  if (!timeout) {
     return false;
-  });
+  }
+  bool firstTimeout = timeout == timeouts.GetFirst();
+
+  MOZ_LOG(gTimeoutLog, LogLevel::Debug,
+          ("%s(TimeoutManager=%p, timeout=%p, ID=%u)\n",
+           timeout->mReason == Timeout::Reason::eIdleCallbackTimeout
+               ? "CancelIdleCallback"
+               : timeout->mIsInterval ? "ClearInterval" : "ClearTimeout",
+           this, timeout, timeout->mTimeoutId));
+
+  if (timeout->mRunning) {
+    /* We're running from inside the timeout. Mark this
+       timeout for deferred deletion by the code in
+       RunTimeout() */
+    timeout->mIsInterval = false;
+    deferredDeletion = true;
+  } else {
+    /* Delete the aTimeout from the pending aTimeout list */
+    timeout->remove();
+  }
 
   // We don't need to reschedule the executor if any of the following are true:
   //  * If the we weren't cancelling the first timeout, then the executor's
@@ -596,7 +593,7 @@ bool TimeoutManager::ClearTimeoutInternal(int32_t aTimerId,
   //  * If the window has become suspended then we should not start executing
   //    Timeouts.
   if (!firstTimeout || deferredDeletion || mWindow.IsSuspended()) {
-    return cleared;
+    return true;
   }
 
   // Stop the executor and restart it at the next soonest deadline.
@@ -611,7 +608,7 @@ bool TimeoutManager::ClearTimeoutInternal(int32_t aTimerId,
       MOZ_ALWAYS_SUCCEEDS(MaybeSchedule(nextTimeout->When()));
     }
   }
-  return cleared;
+  return true;
 }
 
 void TimeoutManager::RunTimeout(const TimeStamp& aNow,
@@ -1088,6 +1085,7 @@ void TimeoutManager::Timeouts::Insert(Timeout* aTimeout, SortBy aSortBy) {
 
   // Now link in aTimeout after prevSibling.
   if (prevSibling) {
+    aTimeout->SetTimeoutContainer(mTimeouts);
     prevSibling->setNext(aTimeout);
   } else {
     InsertFront(aTimeout);

dom/base/TimeoutManager.h

@@ -139,7 +139,8 @@ class TimeoutManager final {
 
  private:
   struct Timeouts {
-    explicit Timeouts(const TimeoutManager& aManager) : mManager(aManager) {}
+    explicit Timeouts(const TimeoutManager& aManager)
+        : mManager(aManager), mTimeouts(new Timeout::TimeoutSet()) {}
 
     // Insert aTimeout into the list, before all timeouts that would
     // fire after it, but no earlier than the last Timeout with a
@@ -152,9 +153,18 @@ class TimeoutManager final {
     const Timeout* GetLast() const { return mTimeoutList.getLast(); }
     Timeout* GetLast() { return mTimeoutList.getLast(); }
     bool IsEmpty() const { return mTimeoutList.isEmpty(); }
-    void InsertFront(Timeout* aTimeout) { mTimeoutList.insertFront(aTimeout); }
-    void InsertBack(Timeout* aTimeout) { mTimeoutList.insertBack(aTimeout); }
-    void Clear() { mTimeoutList.clear(); }
+    void InsertFront(Timeout* aTimeout) {
+      aTimeout->SetTimeoutContainer(mTimeouts);
+      mTimeoutList.insertFront(aTimeout);
+    }
+    void InsertBack(Timeout* aTimeout) {
+      aTimeout->SetTimeoutContainer(mTimeouts);
+      mTimeoutList.insertBack(aTimeout);
+    }
+    void Clear() {
+      mTimeouts->Clear();
+      mTimeoutList.clear();
+    }
 
     template <class Callable>
     void ForEach(Callable c) {
@@ -176,6 +186,11 @@ class TimeoutManager final {
       return false;
     }
 
+    Timeout* GetTimeout(uint32_t aTimeoutId, Timeout::Reason aReason) {
+      Timeout::TimeoutIdAndReason key = {aTimeoutId, aReason};
+      return mTimeouts->Get(key);
+    }
+
    private:
     // The TimeoutManager that owns this Timeouts structure.  This is
     // mainly used to call state inspecting methods like IsValidFiringId().
@@ -186,6 +201,11 @@ class TimeoutManager final {
     // mTimeoutList is generally sorted by mWhen, but new values are always
     // inserted after any Timeouts with a valid FiringId.
     TimeoutList mTimeoutList;
+
+    // mTimeouts is a set of all the timeouts in the mTimeoutList.
+    // It let's one to have O(1) check whether a timeout id/reason is in the
+    // list.
+    RefPtr<Timeout::TimeoutSet> mTimeouts;
   };
 
   // Each nsGlobalWindowInner object has a TimeoutManager member.  This