Bug 1492930 - Part 3. Expose all frames to image memory reporting. r=tnikkel
☠☠ backed out by 597019fb23d9 ☠ ☠
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 25 Sep 2018 06:18:06 -0400
changeset 438091 587e01daa080af33c43fe71edf307e02c5a97eb2
parent 438090 1a6b422c5a9002dc161e7811bccd6630ccd76d85
child 438092 2959314ecf7caaf5def4a05e280c732c5365027b
push id108222
push useraosmond@gmail.com
push dateTue, 25 Sep 2018 10:18:20 +0000
treeherdermozilla-inbound@9d1ff0d0af47 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1492930
milestone64.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 1492930 - Part 3. Expose all frames to image memory reporting. r=tnikkel At present, surface providers roll up all of their individual surfaces into a single reporting unit. Specifically this means animated image frames are all reported as a block. This patch removes that consolidation and reports every frame as its own SurfaceMemoryReport. This is important because each frame may have its own external image ID, and we want to cross reference that with what we expect from the GPU shared surfaces cache.
image/AnimationSurfaceProvider.cpp
image/AnimationSurfaceProvider.h
image/FrameAnimator.cpp
image/ISurfaceProvider.h
image/Image.h
image/SurfaceCache.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -201,29 +201,33 @@ AnimationSurfaceProvider::LogicalSizeInB
   // animation has, so we really can't do better here. This will become correct
   // once bug 1289954 is complete.
   IntSize size = GetSurfaceKey().Size();
   return 3 * size.width * size.height * sizeof(uint32_t);
 }
 
 void
 AnimationSurfaceProvider::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                                                 size_t& aHeapSizeOut,
-                                                 size_t& aNonHeapSizeOut,
-                                                 size_t& aExtHandlesOut)
+                                                 const AddSizeOfCb& aCallback)
 {
   // Note that the surface cache lock is already held here, and then we acquire
   // mFramesMutex. For this method, this ordering is unavoidable, which means
   // that we must be careful to always use the same ordering elsewhere.
   MutexAutoLock lock(mFramesMutex);
 
+  size_t i = 0;
   for (const RawAccessFrameRef& frame : mFrames.Frames()) {
+    ++i;
     if (frame) {
-      frame->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                    aNonHeapSizeOut, aExtHandlesOut);
+      frame->AddSizeOfExcludingThis(aMallocSizeOf,
+        [&](AddSizeOfCbData& aMetadata) {
+          aMetadata.index = i;
+          aCallback(aMetadata);
+        }
+      );
     }
   }
 }
 
 void
 AnimationSurfaceProvider::Run()
 {
   MutexAutoLock lock(mDecodingMutex);
--- a/image/AnimationSurfaceProvider.h
+++ b/image/AnimationSurfaceProvider.h
@@ -44,19 +44,17 @@ public:
   // We use the ISurfaceProvider constructor of DrawableSurface to indicate that
   // our surfaces are computed lazily.
   DrawableSurface Surface() override { return DrawableSurface(WrapNotNull(this)); }
 
   bool IsFinished() const override;
   bool IsFullyDecoded() const override;
   size_t LogicalSizeInBytes() const override;
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                              size_t& aHeapSizeOut,
-                              size_t& aNonHeapSizeOut,
-                              size_t& aExtHandlesOut) override;
+                              const AddSizeOfCb& aCallback) override;
   void Reset() override;
   void Advance(size_t aFrame) override;
 
 protected:
   DrawableFrameRef DrawableRef(size_t aFrame) override;
   RawAccessFrameRef RawAccessRef(size_t aFrame) override;
 
   // Animation frames are always locked. This is because we only want to release
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -556,30 +556,34 @@ DoCollectSizeOfCompositingSurfaces(const
                                    nsTArray<SurfaceMemoryCounter>& aCounters,
                                    MallocSizeOf aMallocSizeOf)
 {
   // Concoct a SurfaceKey for this surface.
   SurfaceKey key = RasterSurfaceKey(aSurface->GetImageSize(),
                                     DefaultSurfaceFlags(),
                                     PlaybackType::eStatic);
 
-  // Create a counter for this surface.
-  SurfaceMemoryCounter counter(key, /* aIsLocked = */ true,
-                               /* aCannotSubstitute */ false,
-                               /* aIsFactor2 */ false, aType);
+  // Extract the surface's memory usage information.
+  aSurface->AddSizeOfExcludingThis(aMallocSizeOf,
+    [&](imgFrame::AddSizeOfCbData& aMetadata) {
+      // Create a counter for this surface.
+      SurfaceMemoryCounter counter(key, /* aIsLocked = */ true,
+                                   /* aCannotSubstitute */ false,
+                                   /* aIsFactor2 */ false, aType);
 
-  // Extract the surface's memory usage information.
-  size_t heap = 0, nonHeap = 0, handles = 0;
-  aSurface->AddSizeOfExcludingThis(aMallocSizeOf, heap, nonHeap, handles);
-  counter.Values().SetDecodedHeap(heap);
-  counter.Values().SetDecodedNonHeap(nonHeap);
-  counter.Values().SetExternalHandles(handles);
+      // Record it.
+      counter.Values().SetDecodedHeap(aMetadata.heap);
+      counter.Values().SetDecodedNonHeap(aMetadata.nonHeap);
+      counter.Values().SetExternalHandles(aMetadata.handles);
+      counter.Values().SetFrameIndex(aMetadata.index);
+      counter.Values().SetExternalId(aMetadata.externalId);
 
-  // Record it.
-  aCounters.AppendElement(counter);
+      aCounters.AppendElement(counter);
+    }
+  );
 }
 
 void
 FrameAnimator::CollectSizeOfCompositingSurfaces(
     nsTArray<SurfaceMemoryCounter>& aCounters,
     MallocSizeOf aMallocSizeOf) const
 {
   if (mCompositingFrame) {
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -60,31 +60,31 @@ public:
   virtual bool IsFullyDecoded() const { return IsFinished(); }
 
   /// @return the number of bytes of memory this ISurfaceProvider is expected to
   /// require. Optimizations may result in lower real memory usage. Trivial
   /// overhead is ignored. Because this value is used in bookkeeping, it's
   /// important that it be constant over the lifetime of this object.
   virtual size_t LogicalSizeInBytes() const = 0;
 
+  typedef imgFrame::AddSizeOfCbData AddSizeOfCbData;
+  typedef imgFrame::AddSizeOfCb AddSizeOfCb;
+
   /// @return the actual number of bytes of memory this ISurfaceProvider is
   /// using. May vary over the lifetime of the ISurfaceProvider. The default
   /// implementation is appropriate for static ISurfaceProviders.
   virtual void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                                      size_t& aHeapSizeOut,
-                                      size_t& aNonHeapSizeOut,
-                                      size_t& aExtHandlesOut)
+                                      const AddSizeOfCb& aCallback)
   {
     DrawableFrameRef ref = DrawableRef(/* aFrame = */ 0);
     if (!ref) {
       return;
     }
 
-    ref->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                aNonHeapSizeOut, aExtHandlesOut);
+    ref->AddSizeOfExcludingThis(aMallocSizeOf, aCallback);
   }
 
   virtual void Reset() { }
   virtual void Advance(size_t aFrame) { }
 
   /// @return the availability state of this ISurfaceProvider, which indicates
   /// whether DrawableRef() could successfully return a surface. Should only be
   /// called from SurfaceCache code as it relies on SurfaceCache for
--- a/image/Image.h
+++ b/image/Image.h
@@ -31,41 +31,49 @@ class Image;
 
 struct MemoryCounter
 {
   MemoryCounter()
     : mSource(0)
     , mDecodedHeap(0)
     , mDecodedNonHeap(0)
     , mExternalHandles(0)
+    , mFrameIndex(0)
+    , mExternalId(0)
   { }
 
   void SetSource(size_t aCount) { mSource = aCount; }
   size_t Source() const { return mSource; }
   void SetDecodedHeap(size_t aCount) { mDecodedHeap = aCount; }
   size_t DecodedHeap() const { return mDecodedHeap; }
   void SetDecodedNonHeap(size_t aCount) { mDecodedNonHeap = aCount; }
   size_t DecodedNonHeap() const { return mDecodedNonHeap; }
   void SetExternalHandles(size_t aCount) { mExternalHandles = aCount; }
   size_t ExternalHandles() const { return mExternalHandles; }
+  void SetFrameIndex(size_t aIndex) { mFrameIndex = aIndex; }
+  size_t FrameIndex() const { return mFrameIndex; }
+  void SetExternalId(uint64_t aId) { mExternalId = aId; }
+  uint64_t ExternalId() const { return mExternalId; }
 
   MemoryCounter& operator+=(const MemoryCounter& aOther)
   {
     mSource += aOther.mSource;
     mDecodedHeap += aOther.mDecodedHeap;
     mDecodedNonHeap += aOther.mDecodedNonHeap;
     mExternalHandles += aOther.mExternalHandles;
     return *this;
   }
 
 private:
   size_t mSource;
   size_t mDecodedHeap;
   size_t mDecodedNonHeap;
   size_t mExternalHandles;
+  size_t mFrameIndex;
+  uint64_t mExternalId;
 };
 
 enum class SurfaceMemoryCounterType
 {
   NORMAL,
   COMPOSITING,
   COMPOSITING_PREV
 };
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -183,39 +183,40 @@ public:
     SurfaceMemoryReport(nsTArray<SurfaceMemoryCounter>& aCounters,
                         MallocSizeOf                    aMallocSizeOf)
       : mCounters(aCounters)
       , mMallocSizeOf(aMallocSizeOf)
     { }
 
     void Add(NotNull<CachedSurface*> aCachedSurface, bool aIsFactor2)
     {
-      SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
-                                   aCachedSurface->IsLocked(),
-                                   aCachedSurface->CannotSubstitute(),
-                                   aIsFactor2);
-
       if (aCachedSurface->IsPlaceholder()) {
         return;
       }
 
       // Record the memory used by the ISurfaceProvider. This may not have a
       // straightforward relationship to the size of the surface that
       // DrawableRef() returns if the surface is generated dynamically. (i.e.,
       // for surfaces with PlaybackType::eAnimated.)
-      size_t heap = 0;
-      size_t nonHeap = 0;
-      size_t handles = 0;
-      aCachedSurface->mProvider
-        ->AddSizeOfExcludingThis(mMallocSizeOf, heap, nonHeap, handles);
-      counter.Values().SetDecodedHeap(heap);
-      counter.Values().SetDecodedNonHeap(nonHeap);
-      counter.Values().SetExternalHandles(handles);
+      aCachedSurface->mProvider->AddSizeOfExcludingThis(mMallocSizeOf,
+        [&](ISurfaceProvider::AddSizeOfCbData& aMetadata) {
+          SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
+                                       aCachedSurface->IsLocked(),
+                                       aCachedSurface->CannotSubstitute(),
+                                       aIsFactor2);
 
-      mCounters.AppendElement(counter);
+          counter.Values().SetDecodedHeap(aMetadata.heap);
+          counter.Values().SetDecodedNonHeap(aMetadata.nonHeap);
+          counter.Values().SetExternalHandles(aMetadata.handles);
+          counter.Values().SetFrameIndex(aMetadata.index);
+          counter.Values().SetExternalId(aMetadata.externalId);
+
+          mCounters.AppendElement(counter);
+        }
+      );
     }
 
   private:
     nsTArray<SurfaceMemoryCounter>& mCounters;
     MallocSizeOf                    mMallocSizeOf;
   };
 
 private:
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -937,32 +937,34 @@ void
 imgFrame::SetCompositingFailed(bool val)
 {
   MOZ_ASSERT(NS_IsMainThread());
   mCompositingFailed = val;
 }
 
 void
 imgFrame::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
-                                 size_t& aHeapSizeOut,
-                                 size_t& aNonHeapSizeOut,
-                                 size_t& aExtHandlesOut) const
+                                 const AddSizeOfCb& aCallback) const
 {
   MonitorAutoLock lock(mMonitor);
 
+  AddSizeOfCbData metadata;
   if (mPalettedImageData) {
-    aHeapSizeOut += aMallocSizeOf(mPalettedImageData);
+    metadata.heap += aMallocSizeOf(mPalettedImageData);
   }
   if (mLockedSurface) {
-    aHeapSizeOut += aMallocSizeOf(mLockedSurface);
+    metadata.heap += aMallocSizeOf(mLockedSurface);
   }
   if (mOptSurface) {
-    aHeapSizeOut += aMallocSizeOf(mOptSurface);
+    metadata.heap += aMallocSizeOf(mOptSurface);
   }
   if (mRawSurface) {
-    aHeapSizeOut += aMallocSizeOf(mRawSurface);
-    mRawSurface->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                        aNonHeapSizeOut, aExtHandlesOut);
+    metadata.heap += aMallocSizeOf(mRawSurface);
+    mRawSurface->AddSizeOfExcludingThis(aMallocSizeOf, metadata.heap,
+                                        metadata.nonHeap, metadata.handles,
+                                        metadata.externalId);
   }
+
+  aCallback(metadata);
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -201,19 +201,32 @@ public:
   bool GetCompositingFailed() const;
   void SetCompositingFailed(bool val);
 
   void SetOptimizable();
 
   void FinalizeSurface();
   already_AddRefed<SourceSurface> GetSourceSurface();
 
-  void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut,
-                              size_t& aNonHeapSizeOut,
-                              size_t& aExtHandlesOut) const;
+  struct AddSizeOfCbData {
+    AddSizeOfCbData()
+      : heap(0), nonHeap(0), handles(0), index(0), externalId(0)
+    { }
+
+    size_t heap;
+    size_t nonHeap;
+    size_t handles;
+    size_t index;
+    uint64_t externalId;
+  };
+
+  typedef std::function<void(AddSizeOfCbData& aMetadata)> AddSizeOfCb;
+
+  void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
+                              const AddSizeOfCb& aCallback) const;
 
 private: // methods
 
   ~imgFrame();
 
   /**
    * Used when the caller desires raw access to the underlying frame buffer.
    * If the locking succeeds, the data pointer to the start of the buffer is