Bug 1371882 - MemoryBlockCache is responsible for tracking the combined size of all its buffers - r?cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Thu, 15 Jun 2017 17:10:54 +1200
changeset 595169 2994a8575637e5fb4c894221360c7868a5f1f9d1
parent 595168 6da7c3e9504cd58c6d3afc656ae58b9ab445db5e
child 595170 e3276ec587259bedda46b99500a65c7b9d5abed2
push id64265
push usergsquelart@mozilla.com
push dateFri, 16 Jun 2017 03:37:56 +0000
reviewerscpearce
bugs1371882
milestone56.0a1
Bug 1371882 - MemoryBlockCache is responsible for tracking the combined size of all its buffers - r?cpearce MemoryBlockCache won't allow initializing, or growing an existing buffer, above the limit (min of 'media.memory_caches_combined_limit_kb' or sysmem*'media.memory_caches_combined_limit_pc_sysmem'/100). MozReview-Commit-ID: 6MkwFp2eeth
dom/media/MediaCache.cpp
dom/media/MemoryBlockCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -18,17 +18,16 @@
 #include "MemoryBlockCache.h"
 #include "nsIObserverService.h"
 #include "nsISeekableStream.h"
 #include "nsIPrincipal.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Telemetry.h"
-#include "prsystem.h"
 #include <algorithm>
 
 namespace mozilla {
 
 #undef LOG
 #undef LOGI
 LazyLogModule gMediaCacheLog("MediaCache");
 #define LOG(...) MOZ_LOG(gMediaCacheLog, LogLevel::Debug, (__VA_ARGS__))
@@ -266,48 +265,40 @@ protected:
     MOZ_COUNT_CTOR(MediaCache);
     MediaCacheFlusher::RegisterMediaCache(this);
   }
 
   ~MediaCache()
   {
     NS_ASSERTION(NS_IsMainThread(), "Only destroy MediaCache on main thread");
     if (this == gMediaCache) {
+      LOG("~MediaCache(Global file-backed MediaCache)");
       // This is the file-backed MediaCache, reset the global pointer.
       gMediaCache = nullptr;
-    }
-    if (mContentLength > 0) {
-      // This is an memory-backed MediaCache, update the combined memory usage.
-      gMediaMemoryCachesCombinedSize -= mContentLength;
-      LOG("~MediaCache(Memory-backed MediaCache %p) -> combined size now %" PRIi64,
-          this,
-          gMediaMemoryCachesCombinedSize);
-    } else {
-      LOG("~MediaCache(Global file-backed MediaCache)");
-    }
-    MediaCacheFlusher::UnregisterMediaCache(this);
-    NS_ASSERTION(mStreams.IsEmpty(), "Stream(s) still open!");
-    Truncate();
-    NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
-    if (mContentLength <= 0) {
       // Only gather "MEDIACACHE" telemetry for the file-based cache.
       LOG("MediaCache::~MediaCache(this=%p) MEDIACACHE_WATERMARK_KB=%u",
           this,
           unsigned(mIndexWatermark * MediaCache::BLOCK_SIZE / 1024));
       Telemetry::Accumulate(
         Telemetry::HistogramID::MEDIACACHE_WATERMARK_KB,
         uint32_t(mIndexWatermark * MediaCache::BLOCK_SIZE / 1024));
       LOG(
         "MediaCache::~MediaCache(this=%p) MEDIACACHE_BLOCKOWNERS_WATERMARK=%u",
         this,
         unsigned(mBlockOwnersWatermark));
       Telemetry::Accumulate(
         Telemetry::HistogramID::MEDIACACHE_BLOCKOWNERS_WATERMARK,
         mBlockOwnersWatermark);
+    } else {
+      LOG("~MediaCache(Memory-backed MediaCache %p)", this);
     }
+    MediaCacheFlusher::UnregisterMediaCache(this);
+    NS_ASSERTION(mStreams.IsEmpty(), "Stream(s) still open!");
+    Truncate();
+    NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
 
     MOZ_COUNT_DTOR(MediaCache);
   }
 
   // Find a free or reusable block and return its index. If there are no
   // free blocks and no reusable blocks, add a new block to the cache
   // and return it. Can return -1 on OOM.
   int32_t FindBlockForIncomingData(TimeStamp aNow, MediaCacheStream* aStream);
@@ -397,19 +388,16 @@ protected:
   void Truncate();
 
   // There is at most one file-backed media cache.
   // It is owned by all MediaCacheStreams that use it.
   // This is a raw pointer set by GetMediaCache(), and reset by ~MediaCache(),
   // both on the main thread; and is not accessed anywhere else.
   static MediaCache* gMediaCache;
 
-  // Combined size of all memory-backed MediaCaches. Main-thread only.
-  static int64_t gMediaMemoryCachesCombinedSize;
-
   // Expected content length if known initially from the HTTP Content-Length
   // header (this is a memory-backed MediaCache), otherwise -1 (file-backed
   // MediaCache).
   const int64_t mContentLength;
 
   // This member is main-thread only. It's used to allocate unique
   // resource IDs to streams.
   int64_t                       mNextResourceID;
@@ -437,18 +425,16 @@ protected:
   bool            mInUpdate;
 #endif
   // A list of resource IDs to notify about the change in suspended status.
   nsTArray<int64_t> mSuspendedStatusToNotify;
 };
 
 // Initialized to nullptr by non-local static initialization.
 /* static */ MediaCache* MediaCache::gMediaCache;
-// Initialized to 0 by non-local static initialization.
-/* static */ int64_t MediaCache::gMediaMemoryCachesCombinedSize;
 
 NS_IMETHODIMP
 MediaCacheFlusher::Observe(nsISupports *aSubject, char const *aTopic, char16_t const *aData)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   if (strcmp(aTopic, "last-pb-context-exited") == 0) {
     for (MediaCache* mc : mMediaCaches) {
@@ -698,36 +684,26 @@ MediaCache::CloseStreamsForPrivateBrowsi
     }
   }
 }
 
 /* static */ RefPtr<MediaCache>
 MediaCache::GetMediaCache(int64_t aContentLength)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
-  static const size_t sysmem = std::max<size_t>(PR_GetPhysicalMemorySize(), 32*1024*1024);
   if (aContentLength > 0 &&
-      aContentLength <=
-        int64_t(MediaPrefs::MediaMemoryCacheMaxSize()) * 1024 &&
-      size_t(gMediaMemoryCachesCombinedSize + aContentLength) <=
-        std::min(
-        size_t(MediaPrefs::MediaMemoryCachesCombinedLimitKb()) * 1024,
-        sysmem * MediaPrefs::MediaMemoryCachesCombinedLimitPcSysmem() / 100)) {
-    // Small-enough resource (and we are under the maximum memory usage), use
-    // a new memory-backed MediaCache.
+      aContentLength <= int64_t(MediaPrefs::MediaMemoryCacheMaxSize()) * 1024) {
+    // Small-enough resource, use a new memory-backed MediaCache.
     RefPtr<MediaBlockCacheBase> bc = new MemoryBlockCache(aContentLength);
     nsresult rv = bc->Init();
     if (NS_SUCCEEDED(rv)) {
       RefPtr<MediaCache> mc = new MediaCache(aContentLength, bc);
-      gMediaMemoryCachesCombinedSize += aContentLength;
-      LOG("GetMediaCache(%" PRIi64
-          ") -> Memory MediaCache %p, combined size %" PRIi64,
+      LOG("GetMediaCache(%" PRIi64 ") -> Memory MediaCache %p",
           aContentLength,
-          mc.get(),
-          gMediaMemoryCachesCombinedSize);
+          mc.get());
       return mc;
     }
     // MemoryBlockCache initialization failed, clean up and try for a
     // file-backed MediaCache below.
   }
 
   if (gMediaCache) {
     LOG("GetMediaCache(%" PRIi64 ") -> Existing file-backed MediaCache",
--- a/dom/media/MemoryBlockCache.cpp
+++ b/dom/media/MemoryBlockCache.cpp
@@ -1,25 +1,34 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 et cindent: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "MemoryBlockCache.h"
 
+#include "MediaPrefs.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/Logging.h"
+#include "prsystem.h"
 
 namespace mozilla {
 
 #undef LOG
 LazyLogModule gMemoryBlockCacheLog("MemoryBlockCache");
 #define LOG(x, ...)                                                            \
   MOZ_LOG(gMemoryBlockCacheLog, LogLevel::Debug, ("%p " x, this, ##__VA_ARGS__))
 
+// Combined sizes of all MemoryBlockCache buffers.
+// Initialized to 0 by non-local static initialization.
+// Increases when a buffer grows (during initialization or unexpected OOB
+// writes), decreases when a MemoryBlockCache (with its buffer) is destroyed.
+static Atomic<size_t> mCombinedSizes;
+
 enum MemoryBlockCacheTelemetryErrors
 {
   // Don't change order/numbers! Add new values at the end and update
   // MEMORYBLOCKCACHE_ERRORS description in Histograms.json.
   InitUnderuse = 0,
   InitAllocation = 1,
   ReadOverrun = 2,
   WriteBlockOverflow = 3,
@@ -41,35 +50,83 @@ MemoryBlockCache::MemoryBlockCache(int64
         this);
     Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
                           InitUnderuse);
   }
 }
 
 MemoryBlockCache::~MemoryBlockCache()
 {
+  size_t sizes = static_cast<size_t>(mCombinedSizes -= mBuffer.Length());
+  LOG("~MemoryBlockCache() - destroying buffer of size %zu; combined sizes now "
+      "%zu",
+      mBuffer.Length(),
+      sizes);
 }
 
 bool
 MemoryBlockCache::EnsureBufferCanContain(size_t aContentLength)
 {
   mMutex.AssertCurrentThreadOwns();
   if (aContentLength == 0) {
     return true;
   }
-  size_t desiredLength = ((aContentLength - 1) / BLOCK_SIZE + 1) * BLOCK_SIZE;
-  if (mBuffer.Length() >= desiredLength) {
+  const size_t initialLength = mBuffer.Length();
+  const size_t desiredLength =
+    ((aContentLength - 1) / BLOCK_SIZE + 1) * BLOCK_SIZE;
+  if (initialLength >= desiredLength) {
     // Already large enough.
     return true;
   }
-  // Need larger buffer, attempt to re-allocate.
-  if (!mBuffer.SetLength(desiredLength, mozilla::fallible)) {
+  // Need larger buffer. If we are allowed more memory, attempt to re-allocate.
+  const size_t extra = desiredLength - initialLength;
+  // Note: There is a small race between testing `atomic + extra > limit` and
+  // committing to it with `atomic += extra` below; but this is acceptable, as
+  // in the worst case it may allow a small number of buffers to go past the
+  // limit.
+  // The alternative would have been to reserve the space first with
+  // `atomic += extra` and then undo it with `atomic -= extra` in case of
+  // failure; but this would have meant potentially preventing other (small but
+  // successful) allocations.
+  static const size_t sysmem =
+    std::max<size_t>(PR_GetPhysicalMemorySize(), 32 * 1024 * 1024);
+  const size_t limit = std::min(
+    size_t(MediaPrefs::MediaMemoryCachesCombinedLimitKb()) * 1024,
+    sysmem * MediaPrefs::MediaMemoryCachesCombinedLimitPcSysmem() / 100);
+  const size_t currentSizes = static_cast<size_t>(mCombinedSizes);
+  if (currentSizes + extra > limit) {
+    LOG("EnsureBufferCanContain(%zu) - buffer size %zu, wanted + %zu = %zu;"
+        " combined sizes %zu + %zu > limit %zu",
+        aContentLength,
+        initialLength,
+        extra,
+        desiredLength,
+        currentSizes,
+        extra,
+        limit);
     return false;
   }
-  mHasGrown = false;
+  if (!mBuffer.SetLength(desiredLength, mozilla::fallible)) {
+    LOG("EnsureBufferCanContain(%zu) - buffer size %zu, wanted + %zu = %zu, "
+        "allocation failed",
+        aContentLength,
+        initialLength,
+        extra,
+        desiredLength);
+    return false;
+  }
+  size_t newSizes = static_cast<size_t>(mCombinedSizes += extra);
+  LOG("EnsureBufferCanContain(%zu) - buffer size %zu + %zu = %zu; combined "
+      "sizes %zu",
+      aContentLength,
+      initialLength,
+      extra,
+      mBuffer.Length(),
+      newSizes);
+  mHasGrown = true;
   return true;
 }
 
 nsresult
 MemoryBlockCache::Init()
 {
   MutexAutoLock lock(mMutex);
   if (mBuffer.IsEmpty()) {