Bug 1286175 - Allow SourceBufferIterator users to specify how many bytes to advance. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Thu, 14 Jul 2016 22:48:31 -0700
changeset 347296 515b58073c4619d3c08391beef75e479b5b896f3
parent 347295 6d6a147955f35e2838c0dd10ee98b5bafef15b1b
child 347297 a6d457ff02be518c5ba3f2d278501e9fbb594069
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1286175
milestone50.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 1286175 - Allow SourceBufferIterator users to specify how many bytes to advance. r=edwin
image/SourceBuffer.cpp
image/SourceBuffer.h
image/StreamingLexer.h
--- a/image/SourceBuffer.cpp
+++ b/image/SourceBuffer.cpp
@@ -42,20 +42,60 @@ SourceBufferIterator::operator=(SourceBu
   mData = aOther.mData;
   mChunkCount = aOther.mChunkCount;
   mByteCount = aOther.mByteCount;
 
   return *this;
 }
 
 SourceBufferIterator::State
-SourceBufferIterator::AdvanceOrScheduleResume(IResumable* aConsumer)
+SourceBufferIterator::AdvanceOrScheduleResume(size_t aRequestedBytes,
+                                              IResumable* aConsumer)
 {
   MOZ_ASSERT(mOwner);
-  return mOwner->AdvanceIteratorOrScheduleResume(*this, aConsumer);
+
+  if (MOZ_UNLIKELY(!HasMore())) {
+    MOZ_ASSERT_UNREACHABLE("Should not advance a completed iterator");
+    return COMPLETE;
+  }
+
+  // The range of data [mOffset, mOffset + mNextReadLength) has just been read
+  // by the caller (or at least they don't have any interest in it), so consume
+  // that data.
+  MOZ_ASSERT(mData.mIterating.mNextReadLength <= mData.mIterating.mAvailableLength);
+  mData.mIterating.mOffset += mData.mIterating.mNextReadLength;
+  mData.mIterating.mAvailableLength -= mData.mIterating.mNextReadLength;
+  mData.mIterating.mNextReadLength = 0;
+
+  if (MOZ_LIKELY(mState == READY)) {
+    // If the caller wants zero bytes of data, that's easy enough; we just
+    // configured ourselves for a zero-byte read above!  In theory we could do
+    // this even in the START state, but it's not important for performance and
+    // breaking the ability of callers to assert that the pointer returned by
+    // Data() is non-null doesn't seem worth it.
+    if (aRequestedBytes == 0) {
+      MOZ_ASSERT(mData.mIterating.mNextReadLength == 0);
+      return READY;
+    }
+
+    // Try to satisfy the request out of our local buffer. This is potentially
+    // much faster than requesting data from our owning SourceBuffer because we
+    // don't have to take the lock. Note that if we have anything at all in our
+    // local buffer, we use it to satisfy the request; @aRequestedBytes is just
+    // the *maximum* number of bytes we can return.
+    if (mData.mIterating.mAvailableLength > 0) {
+      return AdvanceFromLocalBuffer(aRequestedBytes);
+    }
+  }
+
+  // Our local buffer is empty, so we'll have to request data from our owning
+  // SourceBuffer.
+  return mOwner->AdvanceIteratorOrScheduleResume(*this,
+                                                 aRequestedBytes,
+                                                 aConsumer);
 }
 
 bool
 SourceBufferIterator::RemainingBytesIsNoMoreThan(size_t aBytes) const
 {
   MOZ_ASSERT(mOwner);
   return mOwner->RemainingBytesIsNoMoreThan(*this, aBytes);
 }
@@ -517,17 +557,17 @@ SourceBuffer::RemainingBytesIsNoMoreThan
 
   // If the iterator's at the end, the answer is trivial.
   if (!aIterator.HasMore()) {
     return true;
   }
 
   uint32_t iteratorChunk = aIterator.mData.mIterating.mChunk;
   size_t iteratorOffset = aIterator.mData.mIterating.mOffset;
-  size_t iteratorLength = aIterator.mData.mIterating.mLength;
+  size_t iteratorLength = aIterator.mData.mIterating.mAvailableLength;
 
   // Include the bytes the iterator is currently pointing to in the limit, so
   // that the current chunk doesn't have to be a special case.
   size_t bytes = aBytes + iteratorOffset + iteratorLength;
 
   // Count the length over all of our chunks, starting with the one that the
   // iterator is currently pointing to. (This is O(N), but N is expected to be
   // ~1, so it doesn't seem worth caching the length separately.)
@@ -539,24 +579,23 @@ SourceBuffer::RemainingBytesIsNoMoreThan
     }
   }
 
   return true;
 }
 
 SourceBufferIterator::State
 SourceBuffer::AdvanceIteratorOrScheduleResume(SourceBufferIterator& aIterator,
+                                              size_t aRequestedBytes,
                                               IResumable* aConsumer)
 {
   MutexAutoLock lock(mMutex);
 
-  if (MOZ_UNLIKELY(!aIterator.HasMore())) {
-    MOZ_ASSERT_UNREACHABLE("Should not advance a completed iterator");
-    return SourceBufferIterator::COMPLETE;
-  }
+  MOZ_ASSERT(aIterator.HasMore(), "Advancing a completed iterator and "
+                                  "AdvanceOrScheduleResume didn't catch it");
 
   if (MOZ_UNLIKELY(mStatus && NS_FAILED(*mStatus))) {
     // This SourceBuffer is complete due to an error; all reads fail.
     return aIterator.SetComplete(*mStatus);
   }
 
   if (MOZ_UNLIKELY(mChunks.Length() == 0)) {
     // We haven't gotten an initial chunk yet.
@@ -564,32 +603,33 @@ SourceBuffer::AdvanceIteratorOrScheduleR
     return aIterator.SetWaiting();
   }
 
   uint32_t iteratorChunkIdx = aIterator.mData.mIterating.mChunk;
   MOZ_ASSERT(iteratorChunkIdx < mChunks.Length());
 
   const Chunk& currentChunk = mChunks[iteratorChunkIdx];
   size_t iteratorEnd = aIterator.mData.mIterating.mOffset +
-                       aIterator.mData.mIterating.mLength;
+                       aIterator.mData.mIterating.mAvailableLength;
   MOZ_ASSERT(iteratorEnd <= currentChunk.Length());
   MOZ_ASSERT(iteratorEnd <= currentChunk.Capacity());
 
   if (iteratorEnd < currentChunk.Length()) {
     // There's more data in the current chunk.
     return aIterator.SetReady(iteratorChunkIdx, currentChunk.Data(),
-                              iteratorEnd, currentChunk.Length() - iteratorEnd);
+                              iteratorEnd, currentChunk.Length() - iteratorEnd,
+                              aRequestedBytes);
   }
 
   if (iteratorEnd == currentChunk.Capacity() &&
       !IsLastChunk(iteratorChunkIdx)) {
     // Advance to the next chunk.
     const Chunk& nextChunk = mChunks[iteratorChunkIdx + 1];
     return aIterator.SetReady(iteratorChunkIdx + 1, nextChunk.Data(), 0,
-                              nextChunk.Length());
+                              nextChunk.Length(), aRequestedBytes);
   }
 
   MOZ_ASSERT(IsLastChunk(iteratorChunkIdx), "Should've advanced");
 
   if (mStatus) {
     // There's no more data and this SourceBuffer completed successfully.
     MOZ_ASSERT(NS_SUCCEEDED(*mStatus), "Handled failures earlier");
     return aIterator.SetComplete(*mStatus);
--- a/image/SourceBuffer.h
+++ b/image/SourceBuffer.h
@@ -82,17 +82,18 @@ public:
     , mState(START)
     , mChunkCount(0)
     , mByteCount(0)
   {
     MOZ_ASSERT(aOwner);
     mData.mIterating.mChunk = 0;
     mData.mIterating.mData = nullptr;
     mData.mIterating.mOffset = 0;
-    mData.mIterating.mLength = 0;
+    mData.mIterating.mAvailableLength = 0;
+    mData.mIterating.mNextReadLength = 0;
   }
 
   SourceBufferIterator(SourceBufferIterator&& aOther)
     : mOwner(Move(aOther.mOwner))
     , mState(aOther.mState)
     , mData(aOther.mData)
     , mChunkCount(aOther.mChunkCount)
     , mByteCount(aOther.mByteCount)
@@ -104,48 +105,55 @@ public:
 
   /**
    * Returns true if there are no more than @aBytes remaining in the
    * SourceBuffer. If the SourceBuffer is not yet complete, returns false.
    */
   bool RemainingBytesIsNoMoreThan(size_t aBytes) const;
 
   /**
-   * Advances the iterator through the SourceBuffer if possible.
+   * Advances the iterator through the SourceBuffer if possible. Advances no
+   * more than @aRequestedBytes bytes. (Use SIZE_MAX to advance as much as
+   * possible.)
    *
    * This is a wrapper around AdvanceOrScheduleResume() that makes it clearer at
    * the callsite when the no resuming is intended.
    *
    * @return State::READY if the iterator was successfully advanced.
    *         State::WAITING if the iterator could not be advanced because it's
    *           at the end of the underlying SourceBuffer, but the SourceBuffer
    *           may still receive additional data.
    *         State::COMPLETE if the iterator could not be advanced because it's
    *           at the end of the underlying SourceBuffer and the SourceBuffer is
    *           marked complete (i.e., it will never receive any additional
    *           data).
    */
-  State Advance() { return AdvanceOrScheduleResume(nullptr); }
+  State Advance(size_t aRequestedBytes)
+  {
+    return AdvanceOrScheduleResume(aRequestedBytes, nullptr);
+  }
 
   /**
-   * Advances the iterator through the SourceBuffer if possible. If advancing is
-   * not possible and @aConsumer is not null, arranges to call the @aConsumer's
-   * Resume() method when more data is available.
+   * Advances the iterator through the SourceBuffer if possible. Advances no
+   * more than @aRequestedBytes bytes. (Use SIZE_MAX to advance as much as
+   * possible.) If advancing is not possible and @aConsumer is not null,
+   * arranges to call the @aConsumer's Resume() method when more data is
+   * available.
    *
    * @return State::READY if the iterator was successfully advanced.
    *         State::WAITING if the iterator could not be advanced because it's
    *           at the end of the underlying SourceBuffer, but the SourceBuffer
    *           may still receive additional data. @aConsumer's Resume() method
    *           will be called when additional data is available.
    *         State::COMPLETE if the iterator could not be advanced because it's
    *           at the end of the underlying SourceBuffer and the SourceBuffer is
    *           marked complete (i.e., it will never receive any additional
    *           data).
    */
-  State AdvanceOrScheduleResume(IResumable* aConsumer);
+  State AdvanceOrScheduleResume(size_t aRequestedBytes, IResumable* aConsumer);
 
   /// If at the end, returns the status passed to SourceBuffer::Complete().
   nsresult CompletionStatus() const
   {
     MOZ_ASSERT(mState == COMPLETE,
                "Calling CompletionStatus() in the wrong state");
     return mState == COMPLETE ? mData.mAtEnd.mStatus : NS_OK;
   }
@@ -157,49 +165,66 @@ public:
     return mState == READY ? mData.mIterating.mData + mData.mIterating.mOffset
                            : nullptr;
   }
 
   /// If we're ready to read, returns the length of the new data.
   size_t Length() const
   {
     MOZ_ASSERT(mState == READY, "Calling Length() in the wrong state");
-    return mState == READY ? mData.mIterating.mLength : 0;
+    return mState == READY ? mData.mIterating.mNextReadLength : 0;
   }
 
   /// @return a count of the chunks we've advanced through.
   uint32_t ChunkCount() const { return mChunkCount; }
 
   /// @return a count of the bytes in all chunks we've advanced through.
   size_t ByteCount() const { return mByteCount; }
 
 private:
   friend class SourceBuffer;
 
   SourceBufferIterator(const SourceBufferIterator&) = delete;
   SourceBufferIterator& operator=(const SourceBufferIterator&) = delete;
 
   bool HasMore() const { return mState != COMPLETE; }
 
+  State AdvanceFromLocalBuffer(size_t aRequestedBytes)
+  {
+    MOZ_ASSERT(mState == READY, "Advancing in the wrong state");
+    MOZ_ASSERT(mData.mIterating.mAvailableLength > 0,
+               "The local buffer shouldn't be empty");
+    MOZ_ASSERT(mData.mIterating.mNextReadLength == 0,
+               "Advancing without consuming previous data");
+
+    mData.mIterating.mNextReadLength =
+      std::min(mData.mIterating.mAvailableLength, aRequestedBytes);
+
+    return READY;
+  }
+
   State SetReady(uint32_t aChunk, const char* aData,
-                size_t aOffset, size_t aLength)
+                 size_t aOffset, size_t aAvailableLength,
+                 size_t aRequestedBytes)
   {
     MOZ_ASSERT(mState != COMPLETE);
+    mState = READY;
 
     // Update state.
     mData.mIterating.mChunk = aChunk;
     mData.mIterating.mData = aData;
     mData.mIterating.mOffset = aOffset;
-    mData.mIterating.mLength = aLength;
+    mData.mIterating.mAvailableLength = aAvailableLength;
 
     // Update metrics.
     mChunkCount++;
-    mByteCount += aLength;
+    mByteCount += aAvailableLength;
 
-    return mState = READY;
+    // Attempt to advance by the requested number of bytes.
+    return AdvanceFromLocalBuffer(aRequestedBytes);
   }
 
   State SetWaiting()
   {
     MOZ_ASSERT(mState != COMPLETE);
     MOZ_ASSERT(mState != WAITING, "Did we get a spurious wakeup somehow?");
     return mState = WAITING;
   }
@@ -219,17 +244,18 @@ private:
    * states START, READY, and WAITING) and the status the SourceBuffer was
    * completed with if we're in state COMPLETE.
    */
   union {
     struct {
       uint32_t mChunk;
       const char* mData;
       size_t mOffset;
-      size_t mLength;
+      size_t mAvailableLength;
+      size_t mNextReadLength;
     } mIterating;
     struct {
       nsresult mStatus;
     } mAtEnd;
   } mData;
 
   uint32_t mChunkCount;  // Count of chunks we've advanced through.
   size_t mByteCount;     // Count of bytes in all chunks we've advanced through.
@@ -373,16 +399,17 @@ private:
   //////////////////////////////////////////////////////////////////////////////
 
   void AddWaitingConsumer(IResumable* aConsumer);
   void ResumeWaitingConsumers();
 
   typedef SourceBufferIterator::State State;
 
   State AdvanceIteratorOrScheduleResume(SourceBufferIterator& aIterator,
+                                        size_t aRequestedBytes,
                                         IResumable* aConsumer);
   bool RemainingBytesIsNoMoreThan(const SourceBufferIterator& aIterator,
                                   size_t aBytes) const;
 
   void OnIteratorRelease();
 
   //////////////////////////////////////////////////////////////////////////////
   // Helper methods.
--- a/image/StreamingLexer.h
+++ b/image/StreamingLexer.h
@@ -9,16 +9,17 @@
  * image decoders without worrying about the details of how the data is arriving
  * from the network.
  */
 
 #ifndef mozilla_image_StreamingLexer_h
 #define mozilla_image_StreamingLexer_h
 
 #include <algorithm>
+#include <cstdint>
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Variant.h"
 #include "mozilla/Vector.h"
 
 namespace mozilla {
 namespace image {
@@ -269,17 +270,17 @@ public:
   {
     if (mTransition.NextStateIsTerminal()) {
       // We've already reached a terminal state. We never deliver any more data
       // in this case; just return the terminal state again immediately.
       return Some(mTransition.NextStateAsTerminal());
     }
 
     do {
-      switch (aIterator.AdvanceOrScheduleResume(aOnResume)) {
+      switch (aIterator.AdvanceOrScheduleResume(SIZE_MAX, aOnResume)) {
         case SourceBufferIterator::WAITING:
           // We can't continue because the rest of the data hasn't arrived from
           // the network yet. We don't have to do anything special; the
           // SourceBufferIterator will ensure that |aOnResume| gets called when
           // more data is available.
           return Nothing();
 
         case SourceBufferIterator::COMPLETE: