Add CopyOnWriteBuffer class

webrtc/base/buffer.h

Index: webrtc/base/buffer.h
diff --git a/webrtc/base/buffer.h b/webrtc/base/buffer.h
index 5c9380afee03a5e0825d92475e9ed578244314ae..3a40c85eba3615c6145bbc3da20ca2337d8ac9ea 100644
--- a/webrtc/base/buffer.h
+++ b/webrtc/base/buffer.h
@@ -12,16 +12,20 @@
 #define WEBRTC_BASE_BUFFER_H_
 
 #include <algorithm>  // std::swap (pre-C++11)
-#include <cassert>
 #include <cstring>
 #include <memory>
 #include <utility>  // std::swap (C++11 and later)
 
+#include "webrtc/base/checks.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/deprecation.h"
+#include "webrtc/base/refcount.h"
+#include "webrtc/base/scoped_ref_ptr.h"
 
 namespace rtc {
 
+class Buffer;
+
 namespace internal {
 
 // (Internal; please don't use outside this file.) ByteType<T>::t is int if T
@@ -43,10 +47,115 @@ struct ByteType {
   using t = decltype(F(static_cast<T*>(nullptr)));
 };
 
+// Data stored in a buffer. RefCounted and will be shared between buffers that
+// are cloned to avoid unnecessary allocations / copies.
+class BufferData : public RefCountedObject<RefCountInterface> {
+ public:
+  BufferData();
+
+  explicit BufferData(const BufferData& data);
+
+  // Construct data with the specified number of uninitialized bytes.
+  explicit BufferData(size_t size);
+  BufferData(size_t size, size_t capacity);
+
+  // Construct data and copy the specified number of bytes into it.
+  BufferData(const void* data, size_t size);
+  BufferData(const void* data, size_t size, size_t capacity);

[kwiberg-webrtc, 2016/02/14 14:43:46]

Buffer itself has a large number of constructors because it's convenient for
callers. BufferData has only one caller, though, so it makes more sense to
minimize the number of redundant methods and just have the call sites do the
extra work instead. I'm thinking you could probably reduce the number of
constructors to one without harm.

[joachim, 2016/02/14 17:10:16]

Done.

+
+  size_t size() const {
+    RTC_DCHECK(IsConsistent());
+    return size_;
+  }
+  size_t capacity() const {
+    RTC_DCHECK(IsConsistent());
+    return capacity_;
+  }
+  uint8_t* data() const {
+    RTC_DCHECK(IsConsistent());
+    return data_.get();
+  }

[kwiberg-webrtc, 2016/02/14 14:43:46]

Why have all these accessors when you allow Buffer to read private members?

[joachim, 2016/02/14 17:10:16]

Right, the "IsConsistent" checks are already done by their caller. Removed the
accessors.

+
+  void SetData(const void* data, size_t size) {
+    RTC_DCHECK(IsConsistent());
+    size_ = 0;
+    AppendData(data, size);
+  }
+
+  void AppendData(const void* data, size_t size) {
+    RTC_DCHECK(IsConsistent());
+    const size_t new_size = size_ + size;
+    EnsureCapacity(new_size);
+    std::memcpy(data_.get() + size_, data, size);
+    size_ = new_size;
+    RTC_DCHECK(IsConsistent());
+  }
+
+  // Sets the size of the data. If the new size is smaller than the old, the
+  // data contents will be kept but truncated; if the new size is greater,
+  // the existing contents will be kept and the new space will be
+  // uninitialized.
+  void SetSize(size_t size) {
+    EnsureCapacity(size);
+    size_ = size;
+  }
+
+  // Ensure that the data size can be increased to at least capacity without
+  // further reallocation. (Of course, this operation might need to reallocate
+  // the buffer.)
+  void EnsureCapacity(size_t capacity) {
+    RTC_DCHECK(IsConsistent());
+    if (capacity <= capacity_) {
+      return;
+    }
+
+    std::unique_ptr<uint8_t[]> new_data(new uint8_t[capacity]);
+    std::memcpy(new_data.get(), data_.get(), size_);
+    data_ = std::move(new_data);
+    capacity_ = capacity;
+    RTC_DCHECK(IsConsistent());
+  }
+
+  // Resets the data to zero size and capacity.
+  void Clear() {
+    data_.reset();
+    size_ = 0;
+    capacity_ = 0;
+    RTC_DCHECK(IsConsistent());
+  }
+
+ private:
+  friend class rtc::Buffer;

[kwiberg-webrtc, 2016/02/14 14:43:45]

Hmm. Since BufferData is intended to be used only by Buffer, why not make it a
struct without private members (since you apparently don't want to hide its
internals) and put it in the private section of Buffer?

[joachim, 2016/02/14 17:10:16]

Ok, makes sense.

+
+  // Precondition for all methods except Clear and the destructor.
+  // Postcondition for all methods except move construction and move
+  // assignment, which leave the moved-from object in a possibly inconsistent
+  // state.
+  bool IsConsistent() const {
+    return (data_ || capacity_ == 0) && capacity_ >= size_;
+  }
+
+#ifndef NDEBUG
+  // Called when *this has been moved from. Conceptually it's a no-op, but we
+  // can mutate the state slightly to help subsequent sanity checks catch bugs.
+  void OnMovedFrom() {
+    // Ensure that *this is always inconsistent, to provoke bugs.
+    size_ = 1;
+    capacity_ = 0;
+  }
+#endif
+
+  size_t size_;
+  size_t capacity_;
+  std::unique_ptr<uint8_t[]> data_;
+};
+
 }  // namespace internal
 
 // Basic buffer class, can be grown and shrunk dynamically.
 // Unlike std::string/vector, does not initialize data when expanding capacity.
+// The underlying memory is shared between copies of buffers and cloned if a
+// shared buffer is being modified.
 class Buffer {
  public:
   Buffer();                   // An empty buffer.
@@ -65,12 +174,12 @@ class Buffer {
   template <typename T, typename internal::ByteType<T>::t = 0>
   Buffer(const T* data, size_t size, size_t capacity)
       : Buffer(size, capacity) {
-    std::memcpy(data_.get(), data, size);
+    std::memcpy(data_->data(), data, size);
   }
 
   // Construct a buffer from the contents of an array.
   template <typename T, size_t N, typename internal::ByteType<T>::t = 0>
-  Buffer(const T(&array)[N])
+  Buffer(const T(&array)[N])  // NOLINT: runtime/explicit
       : Buffer(array, N) {}
 
   ~Buffer();
@@ -79,42 +188,45 @@ class Buffer {
   // but you may also use .data<int8_t>() and .data<char>().
   template <typename T = uint8_t, typename internal::ByteType<T>::t = 0>
   const T* data() const {
-    assert(IsConsistent());
-    return reinterpret_cast<T*>(data_.get());
+    RTC_DCHECK(IsConsistent());
+    return reinterpret_cast<T*>(data_->data());
   }
+  // Get writable pointer to the data. This will create a copy of the underlying
+  // data if it is shared with other buffers.
   template <typename T = uint8_t, typename internal::ByteType<T>::t = 0>
   T* data() {
-    assert(IsConsistent());
-    return reinterpret_cast<T*>(data_.get());
+    CloneDataIfReferenced();
+    return reinterpret_cast<T*>(data_->data());
   }
 
   size_t size() const {
-    assert(IsConsistent());
-    return size_;
+    RTC_DCHECK(IsConsistent());
+    return data_->size();
   }
   size_t capacity() const {
-    assert(IsConsistent());
-    return capacity_;
+    RTC_DCHECK(IsConsistent());
+    return data_->capacity();
   }
 
   Buffer& operator=(const Buffer& buf) {
-    if (&buf != this)
-      SetData(buf.data(), buf.size());
+    if (&buf != this) {
+      SetData(buf);
+    }
     return *this;
   }
   Buffer& operator=(Buffer&& buf) {
-    assert(IsConsistent());
-    assert(buf.IsConsistent());
-    size_ = buf.size_;
-    capacity_ = buf.capacity_;
-    data_ = std::move(buf.data_);
-    buf.OnMovedFrom();
+    if (&buf != this) {
+      RTC_DCHECK(IsConsistent());
+      RTC_DCHECK(buf.IsConsistent());
+      data_ = std::move(buf.data_);
+      buf.OnMovedFrom();
+    }
     return *this;
   }
 
   bool operator==(const Buffer& buf) const {
-    assert(IsConsistent());
-    return size_ == buf.size() && memcmp(data_.get(), buf.data(), size_) == 0;
+    return data_ == buf.data_ ||
+           (size() == buf.size() && memcmp(data(), buf.data(), size()) == 0);
   }
 
   bool operator!=(const Buffer& buf) const { return !(*this == buf); }
@@ -123,25 +235,30 @@ class Buffer {
   // constructors.
   template <typename T, typename internal::ByteType<T>::t = 0>
   void SetData(const T* data, size_t size) {
-    assert(IsConsistent());
-    size_ = 0;
-    AppendData(data, size);
+    RTC_DCHECK(IsConsistent());
+    if (!data_->HasOneRef()) {
+      data_ = new internal::BufferData(data, size);
+    } else {
+      data_->SetData(data, size);
+    }
   }
   template <typename T, size_t N, typename internal::ByteType<T>::t = 0>
   void SetData(const T(&array)[N]) {
     SetData(array, N);
   }
-  void SetData(const Buffer& buf) { SetData(buf.data(), buf.size()); }
+  void SetData(const Buffer& buf) {
+    if (&buf != this) {
+      RTC_DCHECK(IsConsistent());
+      RTC_DCHECK(buf.IsConsistent());
+      data_ = buf.data_;
+    }
+  }
 
   // Append data to the buffer. Accepts the same types as the constructors.
   template <typename T, typename internal::ByteType<T>::t = 0>
   void AppendData(const T* data, size_t size) {
-    assert(IsConsistent());
-    const size_t new_size = size_ + size;
-    EnsureCapacity(new_size);
-    std::memcpy(data_.get() + size_, data, size);
-    size_ = new_size;
-    assert(IsConsistent());
+    CloneDataIfReferenced();
+    data_->AppendData(data, size);
   }
   template <typename T, size_t N, typename internal::ByteType<T>::t = 0>
   void AppendData(const T(&array)[N]) {
@@ -154,47 +271,43 @@ class Buffer {
   // the existing contents will be kept and the new space will be
   // uninitialized.
   void SetSize(size_t size) {
-    EnsureCapacity(size);
-    size_ = size;
+    CloneDataIfReferenced();
+    data_->SetSize(size);
   }
 
   // Ensure that the buffer size can be increased to at least capacity without
   // further reallocation. (Of course, this operation might need to reallocate
   // the buffer.)
   void EnsureCapacity(size_t capacity) {
-    assert(IsConsistent());
-    if (capacity <= capacity_)
+    RTC_DCHECK(IsConsistent());
+    if (capacity <= data_->capacity())
       return;
-    std::unique_ptr<uint8_t[]> new_data(new uint8_t[capacity]);
-    std::memcpy(new_data.get(), data_.get(), size_);
-    data_ = std::move(new_data);
-    capacity_ = capacity;
-    assert(IsConsistent());
+
+    CloneDataIfReferenced();
+    data_->EnsureCapacity(capacity);
   }
 
   // b.Pass() does the same thing as std::move(b).
   // Deprecated; remove in March 2016 (bug 5373).
   RTC_DEPRECATED Buffer&& Pass() { return DEPRECATED_Pass(); }
   Buffer&& DEPRECATED_Pass() {
-    assert(IsConsistent());
+    RTC_DCHECK(IsConsistent());
     return std::move(*this);
   }
 
-  // Resets the buffer to zero size and capacity. Works even if the buffer has
-  // been moved from.
+  // Resets the buffer to zero size and capacity.
   void Clear() {
-    data_.reset();
-    size_ = 0;
-    capacity_ = 0;
-    assert(IsConsistent());
+    if (!data_->HasOneRef()) {
+      data_ = new internal::BufferData();
+    } else {
+      data_->Clear();
+    }
+    RTC_DCHECK(IsConsistent());
   }
 
-  // Swaps two buffers. Also works for buffers that have been moved from.
+  // Swaps two buffers.
   friend void swap(Buffer& a, Buffer& b) {
-    using std::swap;
-    swap(a.size_, b.size_);
-    swap(a.capacity_, b.capacity_);
-    swap(a.data_, b.data_);
+    std::swap(a.data_, b.data_);
   }
 
  private:
@@ -203,27 +316,31 @@ class Buffer {
   // assignment, which leave the moved-from object in a possibly inconsistent
   // state.
   bool IsConsistent() const {
-    return (data_ || capacity_ == 0) && capacity_ >= size_;
+    return data_ != nullptr && data_->IsConsistent();
   }
 
   // Called when *this has been moved from. Conceptually it's a no-op, but we
   // can mutate the state slightly to help subsequent sanity checks catch bugs.
   void OnMovedFrom() {
-#ifdef NDEBUG
-    // Make *this consistent and empty. Shouldn't be necessary, but better safe
-    // than sorry.
-    size_ = 0;
-    capacity_ = 0;
-#else
+    data_ = new internal::BufferData();
+#ifndef NDEBUG
     // Ensure that *this is always inconsistent, to provoke bugs.
-    size_ = 1;
-    capacity_ = 0;
+    data_->OnMovedFrom();
 #endif
   }
 
-  size_t size_;
-  size_t capacity_;
-  std::unique_ptr<uint8_t[]> data_;
+  // Create a copy of the underlying data if it is referenced from other Buffer
+  // objects.
+  void CloneDataIfReferenced() {
+    RTC_DCHECK(IsConsistent());
+    if (data_->HasOneRef()) {
+      return;
+    }
+
+    data_ = new internal::BufferData(*data_);
+  }
+
+  scoped_refptr<internal::BufferData> data_;
 };
 
 }  // namespace rtc