Backed out changeset 2a95a3663cc2 (bug 923302)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Tue, 25 Nov 2014 14:13:52 +0100
changeset 217474 9bd22a1c116d78c900a1ea6a8d8d69ca89eb7b47
parent 217473 2b255e983a41c223918f434ba9888c3990ed672b
child 217475 8fee160a4abda82ea536706f4c3778ce951dddeb
push id27882
push userryanvm@gmail.com
push dateTue, 25 Nov 2014 21:56:56 +0000
treeherdermozilla-central@ced1402861b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs923302
milestone36.0a1
backs out2a95a3663cc207b654de0db3656e93200cfeba46
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
Backed out changeset 2a95a3663cc2 (bug 923302)
image/src/RasterImage.cpp
image/src/SurfaceCache.cpp
image/src/SurfaceCache.h
image/src/VectorImage.cpp
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -976,23 +976,20 @@ RasterImage::HeapSizeOfSourceWithCompute
   }
   return n;
 }
 
 size_t
 RasterImage::SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation,
                                                      MallocSizeOf aMallocSizeOf) const
 {
-  size_t n = 0;
-  n += SurfaceCache::SizeOfSurfaces(ImageKey(this), aLocation, aMallocSizeOf);
-  if (mFrameBlender) {
-    n += mFrameBlender->SizeOfDecodedWithComputedFallbackIfHeap(aLocation,
-                                                                aMallocSizeOf);
-  }
-  return n;
+  return mFrameBlender
+       ? mFrameBlender->SizeOfDecodedWithComputedFallbackIfHeap(aLocation,
+                                                                aMallocSizeOf)
+       : 0;
 }
 
 size_t
 RasterImage::HeapSizeOfDecodedWithComputedFallback(MallocSizeOf aMallocSizeOf) const
 {
   return SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation::IN_PROCESS_HEAP,
                                                  aMallocSizeOf);
 }
--- a/image/src/SurfaceCache.cpp
+++ b/image/src/SurfaceCache.cpp
@@ -49,30 +49,30 @@ class SurfaceCacheImpl;
 // The single surface cache instance.
 static StaticRefPtr<SurfaceCacheImpl> sInstance;
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // SurfaceCache Implementation
 ///////////////////////////////////////////////////////////////////////////////
 
-/**
+/*
  * Cost models the cost of storing a surface in the cache. Right now, this is
  * simply an estimate of the size of the surface in bytes, but in the future it
  * may be worth taking into account the cost of rematerializing the surface as
  * well.
  */
 typedef size_t Cost;
 
 static Cost ComputeCost(const IntSize& aSize)
 {
   return aSize.width * aSize.height * 4;  // width * height * 4 bytes (32bpp)
 }
 
-/**
+/*
  * Since we want to be able to make eviction decisions based on cost, we need to
  * be able to look up the CachedSurface which has a certain cost as well as the
  * cost associated with a certain CachedSurface. To make this possible, in data
  * structures we actually store a CostEntry, which contains a weak pointer to
  * its associated surface.
  *
  * To make usage of the weak pointer safe, SurfaceCacheImpl always calls
  * StartTracking after a surface is stored in the cache and StopTracking before
@@ -103,17 +103,17 @@ public:
            (mCost == aOther.mCost && mSurface < aOther.mSurface);
   }
 
 private:
   CachedSurface* mSurface;
   Cost           mCost;
 };
 
-/**
+/*
  * A CachedSurface associates a surface with a key that uniquely identifies that
  * surface.
  */
 class CachedSurface
 {
   ~CachedSurface() {}
 public:
   NS_INLINE_DECL_REFCOUNTING(CachedSurface)
@@ -153,57 +153,27 @@ public:
   bool IsLocked() const { return bool(mDrawableRef); }
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
   CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
   Lifetime GetLifetime() const { return mLifetime; }
 
-  // A helper type used by SurfaceCacheImpl::SizeOfSurfacesSum.
-  struct SizeOfSurfacesSum
-  {
-    SizeOfSurfacesSum(gfxMemoryLocation aLocation,
-                      MallocSizeOf      aMallocSizeOf)
-      : mLocation(aLocation)
-      , mMallocSizeOf(aMallocSizeOf)
-      , mSum(0)
-    { }
-
-    void Add(CachedSurface* aCachedSurface)
-    {
-      MOZ_ASSERT(aCachedSurface, "Should have a CachedSurface");
-
-      if (!aCachedSurface->mSurface) {
-        return;
-      }
-
-      mSum += aCachedSurface->mSurface->
-        SizeOfExcludingThisWithComputedFallbackIfHeap(mLocation, mMallocSizeOf);
-    }
-
-    size_t Result() const { return mSum; }
-
-  private:
-    gfxMemoryLocation mLocation;
-    MallocSizeOf      mMallocSizeOf;
-    size_t            mSum;
-  };
-
 private:
   nsExpirationState  mExpirationState;
   nsRefPtr<imgFrame> mSurface;
   DrawableFrameRef   mDrawableRef;
   const Cost         mCost;
   const ImageKey     mImageKey;
   const SurfaceKey   mSurfaceKey;
   const Lifetime     mLifetime;
 };
 
-/**
+/*
  * An ImageSurfaceCache is a per-image surface cache. For correctness we must be
  * able to remove all surfaces associated with an image when the image is
  * destroyed or invalidated. Since this will happen frequently, it makes sense
  * to make it cheap by storing the surfaces for each image separately.
  *
  * ImageSurfaceCache also keeps track of whether its associated image is locked
  * or unlocked.
  */
@@ -252,17 +222,17 @@ public:
   void SetLocked(bool aLocked) { mLocked = aLocked; }
   bool IsLocked() const { return mLocked; }
 
 private:
   SurfaceTable mSurfaces;
   bool         mLocked;
 };
 
-/**
+/*
  * SurfaceCacheImpl is responsible for determining which surfaces will be cached
  * and managing the surface cache data structures. Rather than interact with
  * SurfaceCacheImpl directly, client code interacts with SurfaceCache, which
  * maintains high-level invariants and encapsulates the details of the surface
  * cache's implementation.
  */
 class SurfaceCacheImpl MOZ_FINAL : public nsIMemoryReporter
 {
@@ -575,63 +545,48 @@ public:
     cache->StartTracking(aSurface);
 
     return PL_DHASH_NEXT;
   }
 
   NS_IMETHOD
   CollectReports(nsIHandleReportCallback* aHandleReport,
                  nsISupports*             aData,
-                 bool                     aAnonymize) MOZ_OVERRIDE
+                 bool                     aAnonymize)
   {
-    // We have explicit memory reporting for the surface cache which is more
-    // accurate than the cost metrics we report here, but these metrics are
-    // still useful to report, since they control the cache's behavior.
     nsresult rv;
 
-    rv = MOZ_COLLECT_REPORT("imagelib-surface-cache-estimated-total",
+    rv = MOZ_COLLECT_REPORT("imagelib-surface-cache-total",
                             KIND_OTHER, UNITS_BYTES,
-                            (mMaxCost - mAvailableCost),
-                            "Estimated total memory used by the imagelib "
-                            "surface cache.");
+                            SizeOfSurfacesEstimate(),
+                            "Total memory used by the imagelib surface cache.");
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = MOZ_COLLECT_REPORT("imagelib-surface-cache-estimated-locked",
+    rv = MOZ_COLLECT_REPORT("imagelib-surface-cache-locked",
                             KIND_OTHER, UNITS_BYTES,
-                            mLockedCost,
-                            "Estimated memory used by locked surfaces in the "
-                            "imagelib surface cache.");
+                            SizeOfLockedSurfacesEstimate(),
+                            "Memory used by locked surfaces in the imagelib "
+                            "surface cache.");
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 
-  size_t SizeOfSurfaces(const ImageKey    aImageKey,
-                        gfxMemoryLocation aLocation,
-                        MallocSizeOf      aMallocSizeOf)
+  // XXX(seth): This is currently only an estimate and, since we don't know
+  // which surfaces are in GPU memory and which aren't, it's reported as
+  // KIND_OTHER and will also show up in heap-unclassified. Bug 923302 will
+  // make this nicer.
+  Cost SizeOfSurfacesEstimate() const
   {
-    nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
-    if (!cache) {
-      return 0;  // No surfaces for this image.
-    }
-
-    // Sum the size of all surfaces in the per-image cache.
-    CachedSurface::SizeOfSurfacesSum sum(aLocation, aMallocSizeOf);
-    cache->ForEach(DoSizeOfSurfacesSum, &sum);
-
-    return sum.Result();
+    return mMaxCost - mAvailableCost;
   }
 
-  static PLDHashOperator DoSizeOfSurfacesSum(const SurfaceKey&,
-                                             CachedSurface*    aSurface,
-                                             void*             aSum)
+  Cost SizeOfLockedSurfacesEstimate() const
   {
-    auto sum = static_cast<CachedSurface::SizeOfSurfacesSum*>(aSum);
-    sum->Add(aSurface);
-    return PL_DHASH_NEXT;
+    return mLockedCost;
   }
 
 private:
   already_AddRefed<ImageSurfaceCache> GetImageCache(const ImageKey aImageKey)
   {
     nsRefPtr<ImageSurfaceCache> imageCache;
     mImageCaches.Get(aImageKey, getter_AddRefs(imageCache));
     return imageCache.forget();
@@ -665,19 +620,17 @@ private:
   private:
     SurfaceCacheImpl* const mCache;  // Weak pointer to owner.
   };
 
   struct MemoryPressureObserver : public nsIObserver
   {
     NS_DECL_ISUPPORTS
 
-    NS_IMETHOD Observe(nsISupports*,
-                       const char* aTopic,
-                       const char16_t*) MOZ_OVERRIDE
+    NS_IMETHOD Observe(nsISupports*, const char* aTopic, const char16_t*)
     {
       if (sInstance && strcmp(aTopic, "memory-pressure") == 0) {
         sInstance->DiscardForMemoryPressure();
       }
       return NS_OK;
     }
 
   private:
@@ -838,23 +791,10 @@ SurfaceCache::RemoveImage(Image* aImageK
 SurfaceCache::DiscardAll()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (sInstance) {
     sInstance->DiscardAll();
   }
 }
 
-/* static */ size_t
-SurfaceCache::SizeOfSurfaces(const ImageKey    aImageKey,
-                             gfxMemoryLocation aLocation,
-                             MallocSizeOf      aMallocSizeOf)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!sInstance) {
-    return 0;
-  }
-
-  return sInstance->SizeOfSurfaces(aImageKey, aLocation, aMallocSizeOf);
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/src/SurfaceCache.h
+++ b/image/src/SurfaceCache.h
@@ -6,25 +6,23 @@
 /**
  * SurfaceCache is a service for caching temporary surfaces and decoded image
  * data in imagelib.
  */
 
 #ifndef MOZILLA_IMAGELIB_SURFACECACHE_H_
 #define MOZILLA_IMAGELIB_SURFACECACHE_H_
 
-#include "mozilla/Maybe.h"           // for Maybe
-#include "mozilla/MemoryReporting.h" // for MallocSizeOf
-#include "mozilla/HashFunctions.h"   // for HashGeneric and AddToHash
-#include "gfx2DGlue.h"               // for gfxMemoryLocation
-#include "gfxPoint.h"                // for gfxSize
-#include "nsCOMPtr.h"                // for already_AddRefed
-#include "mozilla/gfx/Point.h"       // for mozilla::gfx::IntSize
-#include "mozilla/gfx/2D.h"          // for SourceSurface
-#include "SVGImageContext.h"         // for SVGImageContext
+#include "mozilla/Maybe.h"          // for Maybe
+#include "mozilla/HashFunctions.h"  // for HashGeneric and AddToHash
+#include "gfxPoint.h"               // for gfxSize
+#include "nsCOMPtr.h"               // for already_AddRefed
+#include "mozilla/gfx/Point.h"      // for mozilla::gfx::IntSize
+#include "mozilla/gfx/2D.h"         // for SourceSurface
+#include "SVGImageContext.h"        // for SVGImageContext
 
 namespace mozilla {
 namespace image {
 
 class DrawableFrameRef;
 class Image;
 class imgFrame;
 
@@ -290,33 +288,16 @@ struct SurfaceCache
    * Evicts all evictable surfaces from the cache.
    *
    * All surfaces are evictable except for persistent surfaces associated with
    * locked images. Non-evictable surfaces can only be removed by
    * RemoveSurface() or RemoveImage().
    */
   static void DiscardAll();
 
-  /**
-   * Computes the size of the surfaces stored for the given image at the given
-   * memory location.
-   *
-   * This is intended for use with memory reporting.
-   *
-   * @param aImageKey     The image to report memory usage for.
-   * @param aLocation     The location (heap, nonheap, etc.) of the memory to
-   *                      report on.
-   * @param aMallocSizeOf A fallback malloc memory reporting function. This
-   *                      should be null unless we're reporting on in-process
-   *                      heap memory.
-   */
-  static size_t SizeOfSurfaces(const ImageKey    aImageKey,
-                               gfxMemoryLocation aLocation,
-                               MallocSizeOf      aMallocSizeOf);
-
 private:
   virtual ~SurfaceCache() = 0;  // Forbid instantiation.
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // MOZILLA_IMAGELIB_SURFACECACHE_H_
--- a/image/src/VectorImage.cpp
+++ b/image/src/VectorImage.cpp
@@ -378,40 +378,35 @@ VectorImage::HeapSizeOfSourceWithCompute
 }
 
 size_t
 VectorImage::HeapSizeOfDecodedWithComputedFallback(MallocSizeOf aMallocSizeOf) const
 {
   // If implementing this, we'll need to restructure our callers to make sure
   // any amount we return is attributed to the vector images measure (i.e.
   // "explicit/images/{content,chrome}/vector/{used,unused}/...")
-  // XXX(seth): Same goes for the other *SizeOfDecoded() methods. We'll do this
-  // in bug 921300 or one of its blockers. For now it seems worthwhile to get
-  // this memory accounted for, even if it gets listed under 'raster'. It does
-  // make some perverse sense, since we are after all reporting on raster data
-  // here - it just happens to be computed from a vector document.
-  return SurfaceCache::SizeOfSurfaces(ImageKey(this),
-                                      gfxMemoryLocation::IN_PROCESS_HEAP,
-                                      aMallocSizeOf);
+  return 0;
 }
 
 size_t
 VectorImage::NonHeapSizeOfDecoded() const
 {
-  return SurfaceCache::SizeOfSurfaces(ImageKey(this),
-                                      gfxMemoryLocation::IN_PROCESS_NONHEAP,
-                                      nullptr);
+  // If implementing this, we'll need to restructure our callers to make sure
+  // any amount we return is attributed to the vector images measure (i.e.
+  // "explicit/images/{content,chrome}/vector/{used,unused}/...")
+  return 0;
 }
 
 size_t
 VectorImage::OutOfProcessSizeOfDecoded() const
 {
-  return SurfaceCache::SizeOfSurfaces(ImageKey(this),
-                                      gfxMemoryLocation::OUT_OF_PROCESS,
-                                      nullptr);
+  // If implementing this, we'll need to restructure our callers to make sure
+  // any amount we return is attributed to the vector images measure (i.e.
+  // "explicit/images/{content,chrome}/vector/{used,unused}/...")
+  return 0;
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF(WindowsMallocSizeOf);
 
 size_t
 VectorImage::HeapSizeOfVectorImageDocument(nsACString* aDocURL) const
 {
   nsIDocument* doc = mSVGDocumentWrapper->GetDocument();