Bug 1643289 - Move elements where possible in nsTPriorityQueue instead of copying them. r=froydnj
authorSimon Giesecke <sgiesecke@mozilla.com>
Wed, 10 Jun 2020 10:46:34 +0000
changeset 534860 f8faae6114e00d12dcfbb3ed07b6fbd89c400944
parent 534859 a3b3ffcc8be73899c90a67489436e3381b4a0f57
child 534861 d5c26d99978e5ffd72406a9a55795eddd03a47c9
push id118585
push usersgiesecke@mozilla.com
push dateWed, 10 Jun 2020 10:57:09 +0000
treeherderautoland@f8faae6114e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1643289
milestone79.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 1643289 - Move elements where possible in nsTPriorityQueue instead of copying them. r=froydnj Also remove redundant check in Push method on result of infallible operation, and change its return type to void. Differential Revision: https://phabricator.services.mozilla.com/D78249
dom/media/platforms/agnostic/DummyMediaDataDecoder.cpp
dom/media/platforms/apple/AppleVTDecoder.cpp
dom/smil/SMILTimeContainer.cpp
dom/smil/SMILTimeContainer.h
xpcom/ds/nsTPriorityQueue.h
--- a/dom/media/platforms/agnostic/DummyMediaDataDecoder.cpp
+++ b/dom/media/platforms/agnostic/DummyMediaDataDecoder.cpp
@@ -38,29 +38,29 @@ RefPtr<MediaDataDecoder::DecodePromise> 
     MediaRawData* aSample) {
   RefPtr<MediaData> data = mCreator->Create(aSample);
 
   if (!data) {
     return DecodePromise::CreateAndReject(NS_ERROR_OUT_OF_MEMORY, __func__);
   }
 
   // Frames come out in DTS order but we need to output them in PTS order.
-  mReorderQueue.Push(data);
+  mReorderQueue.Push(std::move(data));
 
   if (mReorderQueue.Length() > mMaxRefFrames) {
-    return DecodePromise::CreateAndResolve(
-        DecodedData{mReorderQueue.Pop().get()}, __func__);
+    return DecodePromise::CreateAndResolve(DecodedData{mReorderQueue.Pop()},
+                                           __func__);
   }
   return DecodePromise::CreateAndResolve(DecodedData(), __func__);
 }
 
 RefPtr<MediaDataDecoder::DecodePromise> DummyMediaDataDecoder::Drain() {
   DecodedData samples;
   while (!mReorderQueue.IsEmpty()) {
-    samples.AppendElement(mReorderQueue.Pop().get());
+    samples.AppendElement(mReorderQueue.Pop());
   }
   return DecodePromise::CreateAndResolve(std::move(samples), __func__);
 }
 
 RefPtr<MediaDataDecoder::FlushPromise> DummyMediaDataDecoder::Flush() {
   mReorderQueue.Clear();
   return FlushPromise::CreateAndResolve(true, __func__);
 }
--- a/dom/media/platforms/apple/AppleVTDecoder.cpp
+++ b/dom/media/platforms/apple/AppleVTDecoder.cpp
@@ -441,17 +441,17 @@ void AppleVTDecoder::OutputFrame(CVPixel
     MonitorAutoLock mon(mMonitor);
     mPromise.Reject(MediaResult(NS_ERROR_OUT_OF_MEMORY, __func__), __func__);
     return;
   }
 
   // Frames come out in DTS order but we need to output them
   // in composition order.
   MonitorAutoLock mon(mMonitor);
-  mReorderQueue.Push(data);
+  mReorderQueue.Push(std::move(data));
   MaybeResolveBufferedFrames();
 
   LOG("%llu decoded frames queued",
       static_cast<unsigned long long>(mReorderQueue.Length()));
 }
 
 void AppleVTDecoder::OnDecodeError(OSStatus aError) {
   MonitorAutoLock mon(mMonitor);
--- a/dom/smil/SMILTimeContainer.cpp
+++ b/dom/smil/SMILTimeContainer.cpp
@@ -180,25 +180,25 @@ nsresult SMILTimeContainer::SetParent(SM
   nsresult rv = NS_OK;
   if (mParent) {
     rv = mParent->AddChild(*this);
   }
 
   return rv;
 }
 
-bool SMILTimeContainer::AddMilestone(
+void SMILTimeContainer::AddMilestone(
     const SMILMilestone& aMilestone,
     mozilla::dom::SVGAnimationElement& aElement) {
   // We record the milestone time and store it along with the element but this
   // time may change (e.g. if attributes are changed on the timed element in
   // between samples). If this happens, then we may do an unecessary sample
   // but that's pretty cheap.
   MOZ_ASSERT(!mHoldingEntries);
-  return mMilestoneEntries.Push(MilestoneEntry(aMilestone, aElement));
+  mMilestoneEntries.Push(MilestoneEntry(aMilestone, aElement));
 }
 
 void SMILTimeContainer::ClearMilestones() {
   MOZ_ASSERT(!mHoldingEntries);
   mMilestoneEntries.Clear();
 }
 
 bool SMILTimeContainer::GetNextMilestoneInParentTime(
--- a/dom/smil/SMILTimeContainer.h
+++ b/dom/smil/SMILTimeContainer.h
@@ -163,19 +163,18 @@ class SMILTimeContainer {
   nsresult SetParent(SMILTimeContainer* aParent);
 
   /*
    * Registers an element for a sample at the given time.
    *
    * @param   aMilestone  The milestone to register in container time.
    * @param   aElement    The timebase element that needs a sample at
    *                      aMilestone.
-   * @return  true if the element was successfully added, false otherwise.
    */
-  bool AddMilestone(const SMILMilestone& aMilestone,
+  void AddMilestone(const SMILMilestone& aMilestone,
                     mozilla::dom::SVGAnimationElement& aElement);
 
   /*
    * Resets the list of milestones.
    */
   void ClearMilestones();
 
   /*
--- a/xpcom/ds/nsTPriorityQueue.h
+++ b/xpcom/ds/nsTPriorityQueue.h
@@ -57,71 +57,67 @@ class nsTPriorityQueue {
     return mElements[0];
   }
 
   /**
    * Adds an element to the queue
    * @param aElement The element to add
    * @return true on success, false on out of memory.
    */
-  bool Push(const T& aElement) {
-    T* elem = mElements.AppendElement(aElement);
-    if (!elem) {
-      return false;  // Out of memory
-    }
+  void Push(T&& aElement) {
+    mElements.AppendElement(std::move(aElement));
 
     // Sift up
     size_type i = mElements.Length() - 1;
     while (i) {
       size_type parent = (size_type)((i - 1) / 2);
       if (mCompare.LessThan(mElements[parent], mElements[i])) {
         break;
       }
-      Swap(i, parent);
+      std::swap(mElements[i], mElements[parent]);
       i = parent;
     }
-
-    return true;
   }
 
   /**
    * Removes and returns the top-most element from the queue.
    * @return The topmost element, that is, the element 'a' such that there is no
    * other element 'b' in the queue for which Compare(b, a) returns true.
    * @see Top()
    */
   T Pop() {
     MOZ_ASSERT(!mElements.IsEmpty(), "Empty queue");
     T pop = std::move(mElements[0]);
 
-    if (mElements.Length() == 1) {
+    const size_type newLength = mElements.Length() - 1;
+    if (newLength == 0) {
       mElements.Clear();
       return pop;
     }
 
     // Move last to front
     mElements[0] = mElements.PopLastElement();
 
     // Sift down
     size_type i = 0;
-    while (2 * i + 1 < mElements.Length()) {
+    while (2 * i + 1 < newLength) {
       size_type swap = i;
       size_type l_child = 2 * i + 1;
       if (mCompare.LessThan(mElements[l_child], mElements[i])) {
         swap = l_child;
       }
       size_type r_child = l_child + 1;
-      if (r_child < mElements.Length() &&
+      if (r_child < newLength &&
           mCompare.LessThan(mElements[r_child], mElements[swap])) {
         swap = r_child;
       }
       if (swap == i) {
         break;
       }
-      Swap(i, swap);
+      std::swap(mElements[i], mElements[swap]);
       i = swap;
     }
 
     return pop;
   }
 
   /**
    * Removes all elements from the queue.
@@ -139,22 +135,13 @@ class nsTPriorityQueue {
 
   nsTPriorityQueue Clone() const {
     auto res = nsTPriorityQueue{mCompare};
     res.mElements = mElements.Clone();
     return res;
   }
 
  protected:
-  /**
-   * Swaps the elements at the specified indices.
-   */
-  void Swap(size_type aIndexA, size_type aIndexB) {
-    T temp = mElements[aIndexA];
-    mElements[aIndexA] = mElements[aIndexB];
-    mElements[aIndexB] = temp;
-  }
-
   nsTArray<T> mElements;
   Compare mCompare;  // Comparator object
 };
 
 #endif  // NS_TPRIORITY_QUEUE_H_