Bug 1383404 - Part 1. SourceBuffer::Compact should use realloc to grow the first chunk to try to avoid a copy. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 01 Aug 2017 06:59:11 -0400
changeset 420916 4f8e0cb21016059dd19fb4b706e960ec88c64023
parent 420915 0c274a6f7b7b9b41b4b29e34037f4f60244f8d05
child 420917 d8223b2142a9d5834e56f96598d07e9b50843e67
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1383404
milestone56.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 1383404 - Part 1. SourceBuffer::Compact should use realloc to grow the first chunk to try to avoid a copy. r=tnikkel SourceBuffer::Compact attempts to consolidate multiple, discrete allocations into a single buffer, as well as trim excess capacity from a singular allocation if we set aside too much. Using realloc lets jemalloc (or whatever heap implementation we have) decide which is better -- growing the existing buffer if there is sufficient free memory contiguous with the first chunk, or allocating a new buffer entirely. Since we were going to copy regardless, this should result either in an improvement or the status quo. Brief empirical testing on Linux suggests somewhere from 1/3 to 1/2 of allocations resulted in reusing the same data pointer (and presumably avoided a copy as a result). This also has the advantage of potentially reducing OOM errors, as it may have enough room to satisfy an expansion, but not an entirely new buffer.
image/SourceBuffer.cpp
image/SourceBuffer.h
--- a/image/SourceBuffer.cpp
+++ b/image/SourceBuffer.cpp
@@ -219,40 +219,37 @@ SourceBuffer::Compact()
 
   // If our total length is zero (which means ExpectLength() got called, but no
   // data ever actually got written) then just empty our chunk list.
   if (MOZ_UNLIKELY(length == 0)) {
     mChunks.Clear();
     return NS_OK;
   }
 
-  Maybe<Chunk> newChunk = CreateChunk(length, /* aRoundUp = */ false);
-  if (MOZ_UNLIKELY(!newChunk || newChunk->AllocationFailed())) {
-    NS_WARNING("Failed to allocate chunk for SourceBuffer compacting - OOM?");
+  Chunk& mergeChunk = mChunks[0];
+  if (MOZ_UNLIKELY(!mergeChunk.SetCapacity(length))) {
+    NS_WARNING("Failed to reallocate chunk for SourceBuffer compacting - OOM?");
     return NS_OK;
   }
 
-  // Copy our old chunks into the new chunk.
-  for (uint32_t i = 0 ; i < mChunks.Length() ; ++i) {
-    size_t offset = newChunk->Length();
-    MOZ_ASSERT(offset < newChunk->Capacity());
-    MOZ_ASSERT(offset + mChunks[i].Length() <= newChunk->Capacity());
+  // Copy our old chunks into the newly reallocated first chunk.
+  for (uint32_t i = 1 ; i < mChunks.Length() ; ++i) {
+    size_t offset = mergeChunk.Length();
+    MOZ_ASSERT(offset < mergeChunk.Capacity());
+    MOZ_ASSERT(offset + mChunks[i].Length() <= mergeChunk.Capacity());
 
-    memcpy(newChunk->Data() + offset, mChunks[i].Data(), mChunks[i].Length());
-    newChunk->AddLength(mChunks[i].Length());
+    memcpy(mergeChunk.Data() + offset, mChunks[i].Data(), mChunks[i].Length());
+    mergeChunk.AddLength(mChunks[i].Length());
   }
 
-  MOZ_ASSERT(newChunk->Length() == newChunk->Capacity(),
+  MOZ_ASSERT(mergeChunk.Length() == mergeChunk.Capacity(),
              "Compacted chunk has slack space");
 
-  // Replace the old chunks with the new, compact chunk.
-  mChunks.Clear();
-  if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(Move(newChunk))))) {
-    return HandleError(NS_ERROR_OUT_OF_MEMORY);
-  }
+  // Remove the redundant chunks.
+  mChunks.RemoveElementsAt(1, mChunks.Length() - 1);
   mChunks.Compact();
 
   return NS_OK;
 }
 
 /* static */ size_t
 SourceBuffer::RoundedUpCapacity(size_t aCapacity)
 {
--- a/image/SourceBuffer.h
+++ b/image/SourceBuffer.h
@@ -374,69 +374,88 @@ private:
   friend class SourceBufferIterator;
 
   ~SourceBuffer();
 
   //////////////////////////////////////////////////////////////////////////////
   // Chunk type and chunk-related methods.
   //////////////////////////////////////////////////////////////////////////////
 
-  class Chunk
+  class Chunk final
   {
   public:
     explicit Chunk(size_t aCapacity)
       : mCapacity(aCapacity)
       , mLength(0)
     {
       MOZ_ASSERT(aCapacity > 0, "Creating zero-capacity chunk");
-      mData.reset(new (fallible) char[mCapacity]);
+      mData = static_cast<char*>(malloc(mCapacity));
+    }
+
+    ~Chunk()
+    {
+      free(mData);
     }
 
     Chunk(Chunk&& aOther)
       : mCapacity(aOther.mCapacity)
       , mLength(aOther.mLength)
-      , mData(Move(aOther.mData))
+      , mData(aOther.mData)
     {
       aOther.mCapacity = aOther.mLength = 0;
       aOther.mData = nullptr;
     }
 
     Chunk& operator=(Chunk&& aOther)
     {
+      free(mData);
       mCapacity = aOther.mCapacity;
       mLength = aOther.mLength;
-      mData = Move(aOther.mData);
+      mData = aOther.mData;
       aOther.mCapacity = aOther.mLength = 0;
       aOther.mData = nullptr;
       return *this;
     }
 
     bool AllocationFailed() const { return !mData; }
     size_t Capacity() const { return mCapacity; }
     size_t Length() const { return mLength; }
 
     char* Data() const
     {
       MOZ_ASSERT(mData, "Allocation failed but nobody checked for it");
-      return mData.get();
+      return mData;
     }
 
     void AddLength(size_t aAdditionalLength)
     {
       MOZ_ASSERT(mLength + aAdditionalLength <= mCapacity);
       mLength += aAdditionalLength;
     }
 
+    bool SetCapacity(size_t aCapacity)
+    {
+      MOZ_ASSERT(mData, "Allocation failed but nobody checked for it");
+      char* data = static_cast<char*>(realloc(mData, aCapacity));
+      if (!data) {
+        return false;
+      }
+
+      mData = data;
+      mCapacity = aCapacity;
+      return true;
+    }
+
   private:
     Chunk(const Chunk&) = delete;
     Chunk& operator=(const Chunk&) = delete;
 
     size_t mCapacity;
     size_t mLength;
-    UniquePtr<char[]> mData;
+    char* mData;
   };
 
   nsresult AppendChunk(Maybe<Chunk>&& aChunk);
   Maybe<Chunk> CreateChunk(size_t aCapacity, bool aRoundUp = true);
   nsresult Compact();
   static size_t RoundedUpCapacity(size_t aCapacity);
   size_t FibonacciCapacityWithMinimum(size_t aMinCapacity);