Bug 1367538. Clear user data after we're done with the recorder. r=lsalzman
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Wed, 07 Jun 2017 10:28:28 -0400
changeset 413794 5f65427cd949c682d3e47bf370a86011d690c973
parent 413793 4f5ed57189adad562d8885e887f6bd5118d22e77
child 413795 42d138495591207d43ca49e01f1bcb4f23774c17
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1367538
milestone55.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 1367538. Clear user data after we're done with the recorder. r=lsalzman This helps keep us from accumulating all of the recorders. The basic idea is to track weak references to the SourceSurfaces and Fonts that we add UserData to in DrawEventRecorderPrivate and then clear these UserData's when we're done recording. This adds a RemoveAndDestroy helper to UserData to make this possible.
gfx/2d/2D.h
gfx/2d/DrawEventRecorder.h
gfx/2d/DrawTargetRecording.cpp
gfx/2d/UserData.h
gfx/layers/wr/WebRenderPaintedLayerBlob.cpp
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -403,16 +403,19 @@ public:
   }
 
   void AddUserData(UserDataKey *key, void *userData, void (*destroy)(void*)) {
     mUserData.Add(key, userData, destroy);
   }
   void *GetUserData(UserDataKey *key) {
     return mUserData.Get(key);
   }
+  void RemoveUserData(UserDataKey *key) {
+    mUserData.RemoveAndDestroy(key);
+  }
 
 protected:
   friend class DrawTargetCaptureImpl;
   friend class StoredPattern;
 
   // This is for internal use, it ensures the SourceSurface's data remains
   // valid during the lifetime of the SourceSurface.
   // @todo XXX - We need something better here :(. But we may be able to get rid
@@ -807,16 +810,20 @@ public:
 
   void AddUserData(UserDataKey *key, void *userData, void (*destroy)(void*)) {
     mUserData.Add(key, userData, destroy);
   }
   void *GetUserData(UserDataKey *key) {
     return mUserData.Get(key);
   }
 
+  void RemoveUserData(UserDataKey *key) {
+    mUserData.RemoveAndDestroy(key);
+  }
+
   const RefPtr<UnscaledFont>& GetUnscaledFont() const { return mUnscaledFont; }
 
 protected:
   explicit ScaledFont(const RefPtr<UnscaledFont>& aUnscaledFont)
     : mUnscaledFont(aUnscaledFont)
   {}
 
   UserData mUserData;
@@ -1408,16 +1415,17 @@ public:
                                          Color& aColor,
                                          std::vector<Glyph>& aGlyphs) = 0;
 };
 
 class DrawEventRecorder : public RefCounted<DrawEventRecorder>
 {
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DrawEventRecorder)
+  virtual void Finish() = 0;
   virtual ~DrawEventRecorder() { }
 };
 
 struct Tile
 {
   RefPtr<DrawTarget> mDrawTarget;
   IntPoint mTileOrigin;
 };
--- a/gfx/2d/DrawEventRecorder.h
+++ b/gfx/2d/DrawEventRecorder.h
@@ -23,30 +23,59 @@ namespace gfx {
 class PathRecording;
 
 class DrawEventRecorderPrivate : public DrawEventRecorder
 {
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DrawEventRecorderPrivate)
   explicit DrawEventRecorderPrivate(std::ostream *aStream);
   virtual ~DrawEventRecorderPrivate() { }
+  virtual void Finish() {
+    // The iteration is a bit awkward here because our iterator will
+    // be invalidated by the removal
+    for (auto font = mStoredFonts.begin(); font != mStoredFonts.end(); ) {
+      auto oldFont = font++;
+      (*oldFont)->RemoveUserData(reinterpret_cast<UserDataKey*>(this));
+    }
+    for (auto surface = mStoredSurfaces.begin(); surface != mStoredSurfaces.end(); ) {
+      auto oldSurface = surface++;
+      (*oldSurface)->RemoveUserData(reinterpret_cast<UserDataKey*>(this));
+    }
+
+  }
 
   void WriteHeader();
 
   void RecordEvent(const RecordedEvent &aEvent);
   void WritePath(const PathRecording *aPath);
 
   void AddStoredObject(const ReferencePtr aObject) {
     mStoredObjects.insert(aObject);
   }
 
   void RemoveStoredObject(const ReferencePtr aObject) {
     mStoredObjects.erase(aObject);
   }
 
+  void AddScaledFont(ScaledFont* aFont) {
+    mStoredFonts.insert(aFont);
+  }
+
+  void RemoveScaledFont(ScaledFont* aFont) {
+    mStoredFonts.erase(aFont);
+  }
+
+  void AddSourceSurface(SourceSurface* aSurface) {
+    mStoredSurfaces.insert(aSurface);
+  }
+
+  void RemoveSourceSurface(SourceSurface* aSurface) {
+    mStoredSurfaces.erase(aSurface);
+  }
+
   bool HasStoredObject(const ReferencePtr aObject) {
     return mStoredObjects.find(aObject) != mStoredObjects.end();
   }
 
   void AddStoredFontData(const uint64_t aFontDataKey) {
     mStoredFontData.insert(aFontDataKey);
   }
 
@@ -57,23 +86,29 @@ public:
 protected:
   std::ostream *mOutputStream;
 
   virtual void Flush() = 0;
 
 #if defined(_MSC_VER)
   typedef std::unordered_set<const void*> ObjectSet;
   typedef std::unordered_set<uint64_t> Uint64Set;
+  typedef std::unordered_set<ScaledFont*> FontSet;
+  typedef std::unordered_set<SourceSurface*> SurfaceSet;
 #else
   typedef std::set<const void*> ObjectSet;
   typedef std::set<uint64_t> Uint64Set;
+  typedef std::set<ScaledFont*> FontSet;
+  typedef std::set<SourceSurface*> SurfaceSet;
 #endif
 
   ObjectSet mStoredObjects;
   Uint64Set mStoredFontData;
+  FontSet mStoredFonts;
+  SurfaceSet mStoredSurfaces;
 };
 
 class DrawEventRecorderFile : public DrawEventRecorderPrivate
 {
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DrawEventRecorderFile)
   explicit DrawEventRecorderFile(const char *aFilename);
   ~DrawEventRecorderFile();
--- a/gfx/2d/DrawTargetRecording.cpp
+++ b/gfx/2d/DrawTargetRecording.cpp
@@ -23,16 +23,17 @@ struct RecordingSourceSurfaceUserData
   RefPtr<DrawEventRecorderPrivate> recorder;
 };
 
 void RecordingSourceSurfaceUserDataFunc(void *aUserData)
 {
   RecordingSourceSurfaceUserData *userData =
     static_cast<RecordingSourceSurfaceUserData*>(aUserData);
 
+  userData->recorder->RemoveSourceSurface((SourceSurface*)userData->refPtr);
   userData->recorder->RemoveStoredObject(userData->refPtr);
   userData->recorder->RecordEvent(
     RecordedSourceSurfaceDestruction(userData->refPtr));
 
   delete userData;
 }
 
 static void
@@ -61,16 +62,17 @@ EnsureSurfaceStored(DrawEventRecorderPri
 {
   if (aRecorder->HasStoredObject(aSurface)) {
     return;
   }
 
   RefPtr<DataSourceSurface> dataSurf = aSurface->GetDataSurface();
   StoreSourceSurface(aRecorder, aSurface, dataSurf, reason);
   aRecorder->AddStoredObject(aSurface);
+  aRecorder->AddSourceSurface(aSurface);
 
   RecordingSourceSurfaceUserData *userData = new RecordingSourceSurfaceUserData;
   userData->refPtr = aSurface;
   userData->recorder = aRecorder;
   aSurface->AddUserData(reinterpret_cast<UserDataKey*>(aRecorder),
                         userData, &RecordingSourceSurfaceUserDataFunc);
   return;
 }
@@ -370,17 +372,17 @@ struct RecordingFontUserData
 };
 
 void RecordingFontUserDataDestroyFunc(void *aUserData)
 {
   RecordingFontUserData *userData =
     static_cast<RecordingFontUserData*>(aUserData);
 
   userData->recorder->RecordEvent(RecordedScaledFontDestruction(userData->refPtr));
-
+  userData->recorder->RemoveScaledFont((ScaledFont*)userData->refPtr);
   delete userData;
 }
 
 void
 DrawTargetRecording::FillGlyphs(ScaledFont *aFont,
                                 const GlyphBuffer &aBuffer,
                                 const Pattern &aPattern,
                                 const DrawOptions &aOptions,
@@ -416,16 +418,17 @@ DrawTargetRecording::FillGlyphs(ScaledFo
     }
 
     mRecorder->RecordEvent(RecordedScaledFontCreation(aFont, unscaledFont));
 
     RecordingFontUserData *userData = new RecordingFontUserData;
     userData->refPtr = aFont;
     userData->recorder = mRecorder;
     aFont->AddUserData(userDataKey, userData, &RecordingFontUserDataDestroyFunc);
+    userData->recorder->AddScaledFont(aFont);
   }
 
   mRecorder->RecordEvent(RecordedFillGlyphs(this, aFont, aPattern, aOptions, aBuffer.mGlyphs, aBuffer.mNumGlyphs));
   mFinalDT->FillGlyphs(aFont, aBuffer, *AdjustedPattern(aPattern), aOptions, aRenderingOptions);
 }
 
 void
 DrawTargetRecording::Mask(const Pattern &aSource,
--- a/gfx/2d/UserData.h
+++ b/gfx/2d/UserData.h
@@ -67,16 +67,31 @@ public:
           entries[i] = entries[i+1];
         }
         return userData;
       }
     }
     return nullptr;
   }
 
+  /* Remove and destroy a given key */
+  void RemoveAndDestroy(UserDataKey *key)
+  {
+    for (int i=0; i<count; i++) {
+      if (key == entries[i].key) {
+        entries[i].destroy(entries[i].userData);
+        // decrement before looping so entries[i+1] doesn't read past the end:
+        --count;
+        for (;i<count; i++) {
+          entries[i] = entries[i+1];
+        }
+      }
+    }
+  }
+
   /* Retrives the userData for the associated key */
   void *Get(UserDataKey *key) const
   {
     for (int i=0; i<count; i++) {
       if (key == entries[i].key) {
         return entries[i].userData;
       }
     }
--- a/gfx/layers/wr/WebRenderPaintedLayerBlob.cpp
+++ b/gfx/layers/wr/WebRenderPaintedLayerBlob.cpp
@@ -62,16 +62,18 @@ WebRenderPaintedLayerBlob::RenderLayer(w
                                            visibleRegion.ToUnknownRegion(), visibleRegion.ToUnknownRegion(),
                                            DrawRegionClip::DRAW, nsIntRegion(), WrManager()->GetPaintedLayerCallbackData());
 
     if (gfxPrefs::WebRenderHighlightPaintedLayers()) {
       dt->SetTransform(Matrix());
       dt->FillRect(Rect(0, 0, imageSize.width, imageSize.height), ColorPattern(Color(1.0, 0.0, 0.0, 0.5)));
     }
 
+    recorder->Finish();
+
     wr::ByteBuffer bytes;
     bytes.Allocate(recorder->RecordingSize());
     DebugOnly<bool> ok = recorder->CopyRecording((char*)bytes.AsSlice().begin().get(), bytes.AsSlice().length());
     MOZ_ASSERT(ok);
 
     //XXX: We should switch to updating the blob image instead of adding a new one
     //     That will get rid of this discard bit
     if (mImageKey.isSome()) {