Bug 1427639 - Part 2. Fix misleading image memory reporting on Android. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 22 Feb 2018 14:26:29 -0500
changeset 459963 6b4514506318e472a8fb6b2b01ebd115dd0b5ded
parent 459962 99fdbfa4eb210a5bae01f42c7984a4b1a39a70da
child 459964 4b263e3c54ee64f59dc0e33633590fca3fc614e3
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1427639
milestone60.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 1427639 - Part 2. Fix misleading image memory reporting on Android. r=tnikkel The shared memory handle reporting has been generalized to be an external handle reporting. This is used for both shared memory, and for volatile memory (on Android.) This will allow us to have a better sense of just how many handles are being used by images on Android. Additionally we were not properly reporting forced heap allocated memory, if we were putting animated frames on the heap. This is because we used SourceSurfaceAlignedRawData without implementing AddSizeOfExcludingThis.
gfx/2d/2D.h
gfx/2d/SourceSurfaceRawData.cpp
gfx/2d/SourceSurfaceRawData.h
gfx/2d/Tools.h
gfx/layers/SourceSurfaceSharedData.cpp
gfx/layers/SourceSurfaceSharedData.h
gfx/layers/SourceSurfaceVolatileData.cpp
gfx/layers/SourceSurfaceVolatileData.h
image/AnimationSurfaceProvider.cpp
image/AnimationSurfaceProvider.h
image/FrameAnimator.cpp
image/ISurfaceProvider.h
image/Image.h
image/SurfaceCache.cpp
image/imgFrame.cpp
image/imgFrame.h
image/imgLoader.cpp
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -555,17 +555,18 @@ public:
    */
   virtual already_AddRefed<DataSourceSurface> GetDataSurface() override;
 
   /**
    * Add the size of the underlying data buffer to the aggregate.
    */
   virtual void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                       size_t& aHeapSizeOut,
-                                      size_t& aNonHeapSizeOut) const
+                                      size_t& aNonHeapSizeOut,
+                                      size_t& aExtHandlesOut) const
   {
   }
 
   /**
    * Returns whether or not the data was allocated on the heap. This should
    * be used to determine if the memory needs to be cleared to 0.
    */
   virtual bool OnHeap() const
--- a/gfx/2d/SourceSurfaceRawData.cpp
+++ b/gfx/2d/SourceSurfaceRawData.cpp
@@ -35,16 +35,17 @@ SourceSurfaceRawData::InitWrappingData(u
 
 void
 SourceSurfaceRawData::GuaranteePersistance()
 {
   if (mOwnData) {
     return;
   }
 
+  MOZ_ASSERT(!mDeallocator);
   uint8_t* oldData = mRawData;
   mRawData = new uint8_t[mStride * mSize.height];
 
   memcpy(mRawData, oldData, mStride * mSize.height);
   mOwnData = true;
 }
 
 bool
@@ -73,10 +74,19 @@ SourceSurfaceAlignedRawData::Init(const 
   } else {
     mArray.Dealloc();
     mSize.SizeTo(0, 0);
   }
 
   return mArray != nullptr;
 }
 
+void
+SourceSurfaceAlignedRawData::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
+                                                    size_t& aHeapSizeOut,
+                                                    size_t& aNonHeapSizeOut,
+                                                    size_t& aExtHandlesOut) const
+{
+  aHeapSizeOut += mArray.HeapSizeOfExcludingThis(aMallocSizeOf);
+}
+
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/2d/SourceSurfaceRawData.h
+++ b/gfx/2d/SourceSurfaceRawData.h
@@ -123,16 +123,21 @@ public:
 
   virtual uint8_t* GetData() override { return mArray; }
   virtual int32_t Stride() override { return mStride; }
 
   virtual SurfaceType GetType() const override { return SurfaceType::DATA; }
   virtual IntSize GetSize() const override { return mSize; }
   virtual SurfaceFormat GetFormat() const override { return mFormat; }
 
+  void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
+                              size_t& aHeapSizeOut,
+                              size_t& aNonHeapSizeOut,
+                              size_t& aExtHandlesOut) const override;
+
   virtual bool Map(MapType, MappedSurface *aMappedSurface) override
   {
     aMappedSurface->mData = GetData();
     aMappedSurface->mStride = Stride();
     bool success = !!aMappedSurface->mData;
     if (success) {
       mMapCount++;
     }
--- a/gfx/2d/Tools.h
+++ b/gfx/2d/Tools.h
@@ -3,16 +3,17 @@
 /* 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/. */
 
 #ifndef MOZILLA_GFX_TOOLS_H_
 #define MOZILLA_GFX_TOOLS_H_
 
 #include "mozilla/CheckedInt.h"
+#include "mozilla/MemoryReporting.h" // for MallocSizeOf
 #include "mozilla/Move.h"
 #include "mozilla/TypeTraits.h"
 #include "Types.h"
 #include "Point.h"
 #include <math.h>
 
 namespace mozilla {
 namespace gfx {
@@ -225,16 +226,22 @@ struct AlignedArray
 
   void Swap(AlignedArray<T, alignment>& aOther)
   {
     mozilla::Swap(mPtr, aOther.mPtr);
     mozilla::Swap(mStorage, aOther.mStorage);
     mozilla::Swap(mCount, aOther.mCount);
   }
 
+  size_t
+  HeapSizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const
+  {
+    return aMallocSizeOf(mStorage);
+  }
+
   MOZ_ALWAYS_INLINE operator T*()
   {
     return mPtr;
   }
 
   T *mPtr;
 
 private:
--- a/gfx/layers/SourceSurfaceSharedData.cpp
+++ b/gfx/layers/SourceSurfaceSharedData.cpp
@@ -89,21 +89,26 @@ void
 SourceSurfaceSharedData::GuaranteePersistance()
 {
   // Shared memory is not unmapped until we release SourceSurfaceSharedData.
 }
 
 void
 SourceSurfaceSharedData::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                                 size_t& aHeapSizeOut,
-                                                size_t& aNonHeapSizeOut) const
+                                                size_t& aNonHeapSizeOut,
+                                                size_t& aExtHandlesOut) const
 {
+  MutexAutoLock lock(mMutex);
   if (mBuf) {
     aNonHeapSizeOut += GetAlignedDataLength();
   }
+  if (!mClosed) {
+    ++aExtHandlesOut;
+  }
 }
 
 uint8_t*
 SourceSurfaceSharedData::GetDataInternal() const
 {
   mMutex.AssertCurrentThreadOwns();
 
   // If we have an old buffer lingering, it is because we get reallocated to
--- a/gfx/layers/SourceSurfaceSharedData.h
+++ b/gfx/layers/SourceSurfaceSharedData.h
@@ -158,17 +158,18 @@ public:
   SurfaceType GetType() const override { return SurfaceType::DATA_SHARED; }
   IntSize GetSize() const override { return mSize; }
   SurfaceFormat GetFormat() const override { return mFormat; }
 
   void GuaranteePersistance() override;
 
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                               size_t& aHeapSizeOut,
-                              size_t& aNonHeapSizeOut) const override;
+                              size_t& aNonHeapSizeOut,
+                              size_t& aExtHandlesOut) const override;
 
   bool OnHeap() const override
   {
     return false;
   }
 
   /**
    * Although Map (and Moz2D in general) isn't normally threadsafe,
--- a/gfx/layers/SourceSurfaceVolatileData.cpp
+++ b/gfx/layers/SourceSurfaceVolatileData.cpp
@@ -36,18 +36,25 @@ void
 SourceSurfaceVolatileData::GuaranteePersistance()
 {
   MOZ_ASSERT_UNREACHABLE("Should use SourceSurfaceRawData wrapper!");
 }
 
 void
 SourceSurfaceVolatileData::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                                   size_t& aHeapSizeOut,
-                                                  size_t& aNonHeapSizeOut) const
+                                                  size_t& aNonHeapSizeOut,
+                                                  size_t& aExtHandlesOut) const
 {
   if (mVBuf) {
     aHeapSizeOut += mVBuf->HeapSizeOfExcludingThis(aMallocSizeOf);
     aNonHeapSizeOut += mVBuf->NonHeapSizeOfExcludingThis();
+#ifdef ANDROID
+    if (!mVBuf->OnHeap()) {
+      // Volatile buffers keep a file handle open on Android.
+      ++aExtHandlesOut;
+    }
+#endif
   }
 }
 
 } // namespace gfx
 } // namespace mozilla
--- a/gfx/layers/SourceSurfaceVolatileData.h
+++ b/gfx/layers/SourceSurfaceVolatileData.h
@@ -47,17 +47,18 @@ public:
   SurfaceType GetType() const override { return SurfaceType::DATA; }
   IntSize GetSize() const override { return mSize; }
   SurfaceFormat GetFormat() const override { return mFormat; }
 
   void GuaranteePersistance() override;
 
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                               size_t& aHeapSizeOut,
-                              size_t& aNonHeapSizeOut) const override;
+                              size_t& aNonHeapSizeOut,
+                              size_t& aExtHandlesOut) const override;
 
   bool OnHeap() const override
   {
     return mVBuf->OnHeap();
   }
 
   // Althought Map (and Moz2D in general) isn't normally threadsafe,
   // we want to allow it for SourceSurfaceVolatileData since it should
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -109,26 +109,26 @@ AnimationSurfaceProvider::LogicalSizeInB
   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& aSharedHandlesOut)
+                                                 size_t& aExtHandlesOut)
 {
   // 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);
 
   for (const RawAccessFrameRef& frame : mFrames) {
     frame->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                  aNonHeapSizeOut, aSharedHandlesOut);
+                                  aNonHeapSizeOut, aExtHandlesOut);
   }
 }
 
 void
 AnimationSurfaceProvider::Run()
 {
   MutexAutoLock lock(mDecodingMutex);
 
--- a/image/AnimationSurfaceProvider.h
+++ b/image/AnimationSurfaceProvider.h
@@ -43,17 +43,17 @@ public:
   // our surfaces are computed lazily.
   DrawableSurface Surface() override { return DrawableSurface(WrapNotNull(this)); }
 
   bool IsFinished() const override;
   size_t LogicalSizeInBytes() const override;
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                               size_t& aHeapSizeOut,
                               size_t& aNonHeapSizeOut,
-                              size_t& aSharedHandlesOut) override;
+                              size_t& aExtHandlesOut) override;
 
 protected:
   DrawableFrameRef DrawableRef(size_t aFrame) override;
 
   // Animation frames are always locked. This is because we only want to release
   // their memory atomically (due to the surface cache discarding them). If they
   // were unlocked, the OS could end up releasing the memory of random frames
   // from the middle of the animation, which is not worth the complexity of
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -534,17 +534,17 @@ DoCollectSizeOfCompositingSurfaces(const
                                /* 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().SetSharedHandles(handles);
+  counter.Values().SetExternalHandles(handles);
 
   // Record it.
   aCounters.AppendElement(counter);
 }
 
 void
 FrameAnimator::CollectSizeOfCompositingSurfaces(
     nsTArray<SurfaceMemoryCounter>& aCounters,
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -60,25 +60,25 @@ public:
   virtual size_t LogicalSizeInBytes() const = 0;
 
   /// @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& aSharedHandlesOut)
+                                      size_t& aExtHandlesOut)
   {
     DrawableFrameRef ref = DrawableRef(/* aFrame = */ 0);
     if (!ref) {
       return;
     }
 
     ref->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                aNonHeapSizeOut, aSharedHandlesOut);
+                                aNonHeapSizeOut, aExtHandlesOut);
   }
 
   /// @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
   /// synchronization.
   AvailabilityState& Availability() { return mAvailability; }
   const AvailabilityState& Availability() const { return mAvailability; }
--- a/image/Image.h
+++ b/image/Image.h
@@ -31,42 +31,42 @@ class Image;
 ///////////////////////////////////////////////////////////////////////////////
 
 struct MemoryCounter
 {
   MemoryCounter()
     : mSource(0)
     , mDecodedHeap(0)
     , mDecodedNonHeap(0)
-    , mSharedHandles(0)
+    , mExternalHandles(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 SetSharedHandles(size_t aCount) { mSharedHandles = aCount; }
-  size_t SharedHandles() const { return mSharedHandles; }
+  void SetExternalHandles(size_t aCount) { mExternalHandles = aCount; }
+  size_t ExternalHandles() const { return mExternalHandles; }
 
   MemoryCounter& operator+=(const MemoryCounter& aOther)
   {
     mSource += aOther.mSource;
     mDecodedHeap += aOther.mDecodedHeap;
     mDecodedNonHeap += aOther.mDecodedNonHeap;
-    mSharedHandles += aOther.mSharedHandles;
+    mExternalHandles += aOther.mExternalHandles;
     return *this;
   }
 
 private:
   size_t mSource;
   size_t mDecodedHeap;
   size_t mDecodedNonHeap;
-  size_t mSharedHandles;
+  size_t mExternalHandles;
 };
 
 enum class SurfaceMemoryCounterType
 {
   NORMAL,
   COMPOSITING,
   COMPOSITING_PREV
 };
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -202,17 +202,17 @@ public:
       // 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().SetSharedHandles(handles);
+      counter.Values().SetExternalHandles(handles);
 
       mCounters.AppendElement(counter);
     }
 
   private:
     nsTArray<SurfaceMemoryCounter>& mCounters;
     MallocSizeOf                    mMallocSizeOf;
   };
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -945,38 +945,30 @@ imgFrame::SetCompositingFailed(bool val)
   MOZ_ASSERT(NS_IsMainThread());
   mCompositingFailed = val;
 }
 
 void
 imgFrame::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                  size_t& aHeapSizeOut,
                                  size_t& aNonHeapSizeOut,
-                                 size_t& aSharedHandlesOut) const
+                                 size_t& aExtHandlesOut) const
 {
   MonitorAutoLock lock(mMonitor);
 
   if (mPalettedImageData) {
     aHeapSizeOut += aMallocSizeOf(mPalettedImageData);
   }
   if (mLockedSurface) {
     aHeapSizeOut += aMallocSizeOf(mLockedSurface);
   }
   if (mOptSurface) {
     aHeapSizeOut += aMallocSizeOf(mOptSurface);
   }
   if (mRawSurface) {
     aHeapSizeOut += aMallocSizeOf(mRawSurface);
     mRawSurface->AddSizeOfExcludingThis(aMallocSizeOf, aHeapSizeOut,
-                                        aNonHeapSizeOut);
-
-    if (mRawSurface->GetType() == SurfaceType::DATA_SHARED) {
-      auto sharedSurface =
-        static_cast<SourceSurfaceSharedData*>(mRawSurface.get());
-      if (sharedSurface->CanShare()) {
-        ++aSharedHandlesOut;
-      }
-    }
+                                        aNonHeapSizeOut, aExtHandlesOut);
   }
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -245,17 +245,17 @@ public:
 
   void SetOptimizable();
 
   void FinalizeSurface();
   already_AddRefed<SourceSurface> GetSourceSurface();
 
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut,
                               size_t& aNonHeapSizeOut,
-                              size_t& aSharedHandlesOut) const;
+                              size_t& aExtHandlesOut) const;
 
 private: // methods
 
   ~imgFrame();
 
   nsresult LockImageData();
   nsresult UnlockImageData();
   nsresult Optimize(gfx::DrawTarget* aTarget);
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -292,19 +292,19 @@ private:
       if (counter.CannotSubstitute()) {
         surfacePathPrefix.AppendLiteral("cannot_substitute/");
       }
       surfacePathPrefix.AppendLiteral("surface(");
       surfacePathPrefix.AppendInt(counter.Key().Size().width);
       surfacePathPrefix.AppendLiteral("x");
       surfacePathPrefix.AppendInt(counter.Key().Size().height);
 
-      if (counter.Values().SharedHandles() > 0) {
-        surfacePathPrefix.AppendLiteral(", shared:");
-        surfacePathPrefix.AppendInt(uint32_t(counter.Values().SharedHandles()));
+      if (counter.Values().ExternalHandles() > 0) {
+        surfacePathPrefix.AppendLiteral(", external:");
+        surfacePathPrefix.AppendInt(uint32_t(counter.Values().ExternalHandles()));
       }
 
       if (counter.Type() == SurfaceMemoryCounterType::NORMAL) {
         PlaybackType playback = counter.Key().Playback();
         surfacePathPrefix.Append(playback == PlaybackType::eAnimated
                                  ? " (animation)"
                                  : "");