Bug 1541105 - Cache2 I/O thread may do excessive number of AddRef/Release calls on queued nsCOMPtr's under a lock, blocking main thread under heavy load; use nsTArray APIs allowing mere move of the elements instead, r=michal,froydnj
authorHonza Bambas <honzab.moz@firemni.cz>
Wed, 03 Apr 2019 11:27:28 +0000
changeset 467756 d71df181f3d948fa113de06223649f5e6298b47b
parent 467755 2d7fcd115d9fd9e5cf19b40dcf634d885dc3d2df
child 467757 60c20a0f320cc769a8da229cdf6edfbbf38d3ed3
push id112658
push useraciure@mozilla.com
push dateThu, 04 Apr 2019 04:41:45 +0000
treeherdermozilla-inbound@a36718c8163e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal, froydnj
bugs1541105
milestone68.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 1541105 - Cache2 I/O thread may do excessive number of AddRef/Release calls on queued nsCOMPtr's under a lock, blocking main thread under heavy load; use nsTArray APIs allowing mere move of the elements instead, r=michal,froydnj Differential Revision: https://phabricator.services.mozilla.com/D25815
netwerk/cache2/CacheIOThread.cpp
--- a/netwerk/cache2/CacheIOThread.cpp
+++ b/netwerk/cache2/CacheIOThread.cpp
@@ -557,19 +557,32 @@ void CacheIOThread::LoopOneLevel(uint32_
       ++mEventCounter;
       --mQueueLength[aLevel];
 
       // Release outside the lock.
       events[index] = nullptr;
     }
   }
 
-  if (returnEvents)
-    mEventQueue[aLevel].InsertElementsAt(0, events.Elements() + index,
-                                         length - index);
+  if (returnEvents) {
+    // This code must prevent any AddRef/Release calls on the stored COMPtrs as
+    // it might be exhaustive and block the monitor's lock for an excessive
+    // amout of time.
+
+    // 'index' points at the event that was interrupted and asked for re-run,
+    // all events before have run, been nullified, and can be removed.
+    events.RemoveElementsAt(0, index);
+    // Move events that might have been scheduled on this queue to the tail to
+    // preserve the expected per-queue FIFO order.
+    if (!events.AppendElements(std::move(mEventQueue[aLevel]))) {
+      MOZ_CRASH("Can't allocate memory for cache IO thread queue");
+    }
+    // And finally move everything back to the main queue.
+    events.SwapElements(mEventQueue[aLevel]);
+  }
 }
 
 bool CacheIOThread::EventsPending(uint32_t aLastLevel) {
   return mLowestLevelWaiting < aLastLevel || mHasXPCOMEvents;
 }
 
 NS_IMETHODIMP CacheIOThread::OnDispatchedEvent() {
   MonitorAutoLock lock(mMonitor);