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.
dothq · Sep 25, 2018 · d49646b · d49646b
1 parent c1c4901
commit d49646b
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 53 deletions.
diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp
@@ -206,19 +206,23 @@ AnimationSurfaceProvider::LogicalSizeInBytes() const
 
 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);
+        }
+      );
     }
   }
 }

diff --git a/image/AnimationSurfaceProvider.h b/image/AnimationSurfaceProvider.h
@@ -49,9 +49,7 @@ class AnimationSurfaceProvider final
   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;
 

diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp
@@ -561,20 +561,24 @@ DoCollectSizeOfCompositingSurfaces(const RawAccessFrameRef& aSurface,
                                     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.
-  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.
-  aCounters.AppendElement(counter);
+  aSurface->AddSizeOfExcludingThis(aMallocSizeOf,
+    [&](imgFrame::AddSizeOfCbData& aMetadata) {
+      // Create a counter for this surface.
+      SurfaceMemoryCounter counter(key, /* aIsLocked = */ true,
+                                   /* aCannotSubstitute */ false,
+                                   /* aIsFactor2 */ false, aType);
+
+      // 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);
+
+      aCounters.AppendElement(counter);
+    }
+  );
 }
 
 void

diff --git a/image/ISurfaceProvider.h b/image/ISurfaceProvider.h
@@ -65,21 +65,21 @@ class ISurfaceProvider
   /// 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() { }

diff --git a/image/Image.h b/image/Image.h
@@ -36,6 +36,8 @@ struct MemoryCounter
     , mDecodedHeap(0)
     , mDecodedNonHeap(0)
     , mExternalHandles(0)
+    , mFrameIndex(0)
+    , mExternalId(0)
   { }
 
   void SetSource(size_t aCount) { mSource = aCount; }
@@ -46,6 +48,10 @@ struct MemoryCounter
   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)
   {
@@ -61,6 +67,8 @@ struct MemoryCounter
   size_t mDecodedHeap;
   size_t mDecodedNonHeap;
   size_t mExternalHandles;
+  size_t mFrameIndex;
+  uint64_t mExternalId;
 };
 
 enum class SurfaceMemoryCounterType

diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp
@@ -188,11 +188,6 @@ class CachedSurface
 
     void Add(NotNull<CachedSurface*> aCachedSurface, bool aIsFactor2)
     {
-      SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
-                                   aCachedSurface->IsLocked(),
-                                   aCachedSurface->CannotSubstitute(),
-                                   aIsFactor2);
-
       if (aCachedSurface->IsPlaceholder()) {
         return;
       }
@@ -201,16 +196,22 @@ class CachedSurface
       // 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);
-
-      mCounters.AppendElement(counter);
+      aCachedSurface->mProvider->AddSizeOfExcludingThis(mMallocSizeOf,
+        [&](ISurfaceProvider::AddSizeOfCbData& aMetadata) {
+          SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
+                                       aCachedSurface->IsLocked(),
+                                       aCachedSurface->CannotSubstitute(),
+                                       aIsFactor2);
+
+          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:

diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp
@@ -942,26 +942,28 @@ imgFrame::SetCompositingFailed(bool 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

diff --git a/image/imgFrame.h b/image/imgFrame.h
@@ -206,9 +206,22 @@ class imgFrame
   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