Bug 1412737. P2 - Read() should return only when enough bytes are read or EOS/error is encountered. r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Wed, 01 Nov 2017 16:53:29 +0800
changeset 444484 39f2cccbf9f73a3c0ea7dc6e2fe72a0216418b03
parent 444483 5a5808da985bb0ff2cc705dd83feb54323cb0a17
child 444485 95b93405af2b238e0828171eb8f36b3b9869c8cf
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbechen, gerald
bugs1412737
milestone58.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 1412737. P2 - Read() should return only when enough bytes are read or EOS/error is encountered. r=bechen,gerald This will remove the need to retry reading for the callers. Note since data is usually downloaded faster than being consumed, we don't benefit much in reading data from a partial block in the memory. Chances are we still need to wait for the block to commit to the cache so the reader can continue. So we change the code to always read data from the cache or from the last block when it is completed (reaching EOS). This change allows up to somewhat optimize NotifyDataReceived() which won't have to wake up readers if no blocks are committed to the cache. MozReview-Commit-ID: KwgNSOawuAE
dom/media/MediaCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2550,51 +2550,49 @@ MediaCacheStream::Read(char* aBuffer, ui
       size = std::min(size, int64_t(INT32_MAX));
     }
 
     int32_t cacheBlock =
       size_t(streamBlock) < mBlocks.Length() ? mBlocks[streamBlock] : -1;
     if (cacheBlock < 0) {
       // We don't have a complete cached block here.
 
-      if (count > 0) {
-        // Some data has been read, so return what we've got instead of
-        // blocking or trying to find a stream with a partial block.
-        break;
-      }
-
       // See if the data is available in the partial cache block of any
       // stream reading this resource. We need to do this in case there is
       // another stream with this resource that has all the data to the end of
       // the stream but the data doesn't end on a block boundary.
       MediaCacheStream* streamWithPartialBlock = nullptr;
       MediaCache::ResourceStreamIterator iter(mMediaCache, mResourceID);
       while (MediaCacheStream* stream = iter.Next()) {
         if (OffsetToBlockIndexUnchecked(stream->mChannelOffset) ==
               streamBlock &&
-            streamOffset < stream->mChannelOffset) {
+            streamOffset < stream->mChannelOffset &&
+            stream->mChannelOffset == stream->mStreamLength) {
           streamWithPartialBlock = stream;
           break;
         }
       }
       if (streamWithPartialBlock) {
         // We can just use the data in mPartialBlockBuffer. In fact we should
         // use it rather than waiting for the block to fill and land in
         // the cache.
         int64_t bytes = std::min<int64_t>(size, streamWithPartialBlock->mChannelOffset - streamOffset);
         // Clamp bytes until 64-bit file size issues are fixed.
         bytes = std::min(bytes, int64_t(INT32_MAX));
         MOZ_ASSERT(bytes >= 0 && bytes <= aCount, "Bytes out of range.");
-        memcpy(aBuffer,
-          streamWithPartialBlock->mPartialBlockBuffer.get() + offsetInStreamBlock, bytes);
+        memcpy(aBuffer + count,
+               streamWithPartialBlock->mPartialBlockBuffer.get() +
+                 offsetInStreamBlock,
+               bytes);
         if (mCurrentMode == MODE_METADATA) {
           streamWithPartialBlock->mMetadataInPartialBlockBuffer = true;
         }
         streamOffset += bytes;
-        count = bytes;
+        count += bytes;
+        // Break for we've reached EOS and have nothing more to read.
         break;
       }
 
       if (mStreamOffset != streamOffset) {
         // Updat mStreamOffset before we drop the lock. We need to run
         // Update() again since stream reading strategy might have changed.
         mStreamOffset = streamOffset;
         mMediaCache->QueueUpdate();
@@ -2618,23 +2616,26 @@ MediaCacheStream::Read(char* aBuffer, ui
       GetErrorName(rv, name);
       LOGE("Stream %p ReadCacheFile failed, rv=%s", this, name.Data());
       return rv;
     }
     streamOffset += bytes;
     count += bytes;
   }
 
-  if (count > 0) {
-    // Some data was read, so queue an update since block priorities may
-    // have changed
-    mMediaCache->QueueUpdate();
+  *aBytes = count;
+  if (count == 0) {
+    return NS_OK;
   }
+
+  // Some data was read, so queue an update since block priorities may
+  // have changed
+  mMediaCache->QueueUpdate();
+
   LOG("Stream %p Read at %" PRId64 " count=%d", this, streamOffset-count, count);
-  *aBytes = count;
   mStreamOffset = streamOffset;
   return NS_OK;
 }
 
 nsresult
 MediaCacheStream::ReadAt(int64_t aOffset, char* aBuffer,
                          uint32_t aCount, uint32_t* aBytes)
 {