Optimize the performance of media::ByteQueue.

This changes a few things about the ByteQueue class:
- Growth is now done as std::max(size_needed, 1.25 * size_) in line with
how base::circular_deque grows -- in local tests this seems to yield a
significant memory savings depending on the content.
- In rare cases where there's no used data, the existing allocation is
released before a new one is acquired (to reduce total memory pressure).

BUG=none
TEST=Android bot shows ~1-3% reduction in memory.

Change-Id: Iec376d61bff08fa4723129aed9edb03bb220c7b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691581
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676181}

media/base/byte_queue.cc

@@ -5,17 +5,11 @@
 #include "media/base/byte_queue.h"
 
 #include "base/logging.h"
+#include "base/numerics/checked_math.h"
 
 namespace media {
 
-// Default starting size for the queue.
-enum { kDefaultQueueSize = 1024 };
-
-ByteQueue::ByteQueue()
-    : buffer_(new uint8_t[kDefaultQueueSize]),
-      size_(kDefaultQueueSize),
-      offset_(0),
-      used_(0) {}
+ByteQueue::ByteQueue() : buffer_(new uint8_t[size_]) {}
 
 ByteQueue::~ByteQueue() = default;
 
@@ -28,40 +22,50 @@ void ByteQueue::Push(const uint8_t* data, int size) {
   DCHECK(data);
   DCHECK_GT(size, 0);
 
-  size_t size_needed = used_ + size;
+  // This can never overflow since used and size are both ints.
+  const size_t size_needed = static_cast<size_t>(used_) + size;
 
   // Check to see if we need a bigger buffer.
   if (size_needed > size_) {
-    size_t new_size = 2 * size_;
-    while (size_needed > new_size && new_size > size_)
-      new_size *= 2;
-
-    // Sanity check to make sure we didn't overflow.
-    CHECK_GT(new_size, size_);
-
-    std::unique_ptr<uint8_t[]> new_buffer(new uint8_t[new_size]);
+    // Growth is based on base::circular_deque which grows at 25%.
+    const size_t safe_size =
+        (base::CheckedNumeric<size_t>(size_) + size_ / 4).ValueOrDie();
+    const size_t new_size = std::max(size_needed, safe_size);
 
     // Copy the data from the old buffer to the start of the new one.
-    if (used_ > 0)
-      memcpy(new_buffer.get(), front(), used_);
+    if (used_ > 0) {
+      // Note: We could use realloc() here, but would need an additional move to
+      // pack data at offset_ = 0 after a potential internal new allocation +
+      // copy by realloc().
+      //
+      // In local tests on a few top video sites that ends up being the common
+      // case, so just prefer to copy and pack ourselves.
+      std::unique_ptr<uint8_t[]> new_buffer(new uint8_t[new_size]);
+      memcpy(new_buffer.get(), Front(), used_);
+      buffer_ = std::move(new_buffer);
+    } else {
+      // Free the existing |data| first so that the memory can be reused, if
+      // possible. Note that the new array is purposely not initialized.
+      buffer_.reset();
+      buffer_.reset(new uint8_t[new_size]);
+    }
 
-    buffer_ = std::move(new_buffer);
     size_ = new_size;
     offset_ = 0;
   } else if ((offset_ + used_ + size) > size_) {
     // The buffer is big enough, but we need to move the data in the queue.
-    memmove(buffer_.get(), front(), used_);
+    memmove(buffer_.get(), Front(), used_);
     offset_ = 0;
   }
 
-  memcpy(front() + used_, data, size);
+  memcpy(Front() + used_, data, size);
   used_ += size;
 }
 
 void ByteQueue::Peek(const uint8_t** data, int* size) const {
   DCHECK(data);
   DCHECK(size);
-  *data = front();
+  *data = Front();
   *size = used_;
 }
 
@@ -78,7 +82,7 @@ void ByteQueue::Pop(int count) {
   }
 }
 
-uint8_t* ByteQueue::front() const {
+uint8_t* ByteQueue::Front() const {
   return buffer_.get() + offset_;
 }
 

media/base/byte_queue.h

@@ -15,9 +15,10 @@
 
 namespace media {
 
-// Represents a queue of bytes.
-// Data is added to the end of the queue via an Push() call and removed via
-// Pop(). The contents of the queue can be observed via the Peek() method.
+// Represents a queue of bytes. Data is added to the end of the queue via an
+// Push() call and removed via Pop(). The contents of the queue can be observed
+// via the Peek() method.
+//
 // This class manages the underlying storage of the queue and tries to minimize
 // the number of buffer copies when data is appended and removed.
 class MEDIA_EXPORT ByteQueue {
@@ -31,28 +32,30 @@ class MEDIA_EXPORT ByteQueue {
   // Appends new bytes onto the end of the queue.
   void Push(const uint8_t* data, int size);
 
-  // Get a pointer to the front of the queue and the queue size.
-  // These values are only valid until the next Push() or
-  // Pop() call.
+  // Get a pointer to the front of the queue and the queue size. These values
+  // are only valid until the next Push() or Pop() call.
   void Peek(const uint8_t** data, int* size) const;
 
   // Remove |count| bytes from the front of the queue.
   void Pop(int count);
 
  private:
-  // Returns a pointer to the front of the queue.
-  uint8_t* front() const;
+  // Default starting size for the queue.
+  enum { kDefaultQueueSize = 1024 };
 
-  std::unique_ptr<uint8_t[]> buffer_;
+  // Returns a pointer to the front of the queue.
+  uint8_t* Front() const;
 
   // Size of |buffer_|.
-  size_t size_;
+  size_t size_ = kDefaultQueueSize;
 
   // Offset from the start of |buffer_| that marks the front of the queue.
-  size_t offset_;
+  size_t offset_ = 0u;
 
-  // Number of bytes stored in the queue.
-  int used_;
+  // Number of bytes stored in |buffer_|.
+  int used_ = 0;
+
+  std::unique_ptr<uint8_t[]> buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(ByteQueue);
 };