Bug 1378518 - Better MediaCache GetMaxBlocks() computation - r?jwwang draft
authorGerald Squelart <gsquelart@mozilla.com>
Thu, 06 Jul 2017 16:12:58 +1200
changeset 605132 26e925be52f9eb76219ffc15fa7049d6db50e403
parent 605055 20f32734df750bddada9d1edca665c2ea53946f0
child 605133 f923dd444a34781a30e3f830d82cc3814a225bba
push id67311
push usergsquelart@mozilla.com
push dateFri, 07 Jul 2017 04:13:16 +0000
reviewersjwwang
bugs1378518
milestone56.0a1
Bug 1378518 - Better MediaCache GetMaxBlocks() computation - r?jwwang Retrieve pref from MediaPrefs, which is more efficient than Preferences::GetInt(). Also refactored computations to avoid unnecessary type conversions. MozReview-Commit-ID: Ii27lthRRNI
dom/media/MediaCache.cpp
dom/media/MediaPrefs.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -736,21 +736,35 @@ MediaCache::ReadCacheFile(
       aOffset, reinterpret_cast<uint8_t*>(aData), aLength, aBytes);
   }
 }
 
 static int32_t GetMaxBlocks()
 {
   // We look up the cache size every time. This means dynamic changes
   // to the pref are applied.
-  // Cache size is in KB
-  int32_t cacheSize = Preferences::GetInt("media.cache_size", 500*1024);
-  int64_t maxBlocks = static_cast<int64_t>(cacheSize)*1024/MediaCache::BLOCK_SIZE;
-  maxBlocks = std::max<int64_t>(maxBlocks, 1);
-  return int32_t(std::min<int64_t>(maxBlocks, INT32_MAX));
+  const uint32_t cacheSizeKb =
+    std::min(MediaPrefs::MediaCacheSizeKb(), uint32_t(INT32_MAX) * 2);
+  // Ensure we can divide BLOCK_SIZE by 1024.
+  static_assert(MediaCache::BLOCK_SIZE % 1024 == 0,
+                "BLOCK_SIZE should be a multiple of 1024");
+  // Ensure BLOCK_SIZE/1024 is at least 2.
+  static_assert(MediaCache::BLOCK_SIZE / 1024 >= 2,
+                "BLOCK_SIZE / 1024 should be at least 2");
+  // Ensure we can convert BLOCK_SIZE/1024 to a uint32_t without truncation.
+  static_assert(MediaCache::BLOCK_SIZE / 1024 <= int64_t(UINT32_MAX),
+                "BLOCK_SIZE / 1024 should be at most UINT32_MAX");
+  // Since BLOCK_SIZE is a strict multiple of 1024,
+  // cacheSizeKb * 1024 / BLOCK_SIZE == cacheSizeKb / (BLOCK_SIZE / 1024),
+  // but the latter formula avoids a potential overflow from `* 1024`.
+  // And because BLOCK_SIZE/1024 is at least 2, the maximum cache size
+  // INT32_MAX*2 will give a maxBlocks that can fit in an int32_t.
+  constexpr uint32_t blockSizeKb = uint32_t(MediaCache::BLOCK_SIZE / 1024);
+  const int32_t maxBlocks = int32_t(cacheSizeKb / blockSizeKb);
+  return std::max(maxBlocks, int32_t(1));
 }
 
 int32_t
 MediaCache::FindBlockForIncomingData(TimeStamp aNow,
                                        MediaCacheStream* aStream)
 {
   mReentrantMonitor.AssertCurrentThreadIn();
 
--- a/dom/media/MediaPrefs.h
+++ b/dom/media/MediaPrefs.h
@@ -82,16 +82,17 @@ private:
       AssertMainThread();
       PrefAddVarCache(&mValue, aPreference, mValue);
     }
   };
 
   // This is where DECL_MEDIA_PREF for each of the preferences should go.
 
   // Cache sizes.
+  DECL_MEDIA_PREF("media.cache_size",                         MediaCacheSizeKb, uint32_t, 512000);
   DECL_MEDIA_PREF("media.memory_cache_max_size",              MediaMemoryCacheMaxSize, uint32_t, 8192);
   DECL_MEDIA_PREF("media.memory_caches_combined_limit_kb",    MediaMemoryCachesCombinedLimitKb, uint32_t, 524288);
   DECL_MEDIA_PREF("media.memory_caches_combined_limit_pc_sysmem",
                                                               MediaMemoryCachesCombinedLimitPcSysmem, uint32_t, 5);
   DECL_MEDIA_PREF("media.cache.resource-index",               MediaResourceIndexCache, uint32_t, 8192);
 
   // AudioSink
   DECL_MEDIA_PREF("accessibility.monoaudio.enable",           MonoAudio, bool, false);