From c853597598198f2d4e57abaaa127d16fe5611410 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Mon, 20 Jun 2016 04:47:39 -0700 Subject: [PATCH] rtc::Buffer: Grow capacity by at least 1.5x to prevent quadratic behavior BUG=webrtc:6009 Review-Url: https://codereview.webrtc.org/2078873005 Cr-Commit-Position: refs/heads/master@{#13214} --- webrtc/base/buffer.h | 36 ++++++++++++++++++++++++---------- webrtc/base/buffer_unittest.cc | 4 ++-- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/webrtc/base/buffer.h b/webrtc/base/buffer.h index cdaab544a7..545ddb2be7 100644 --- a/webrtc/base/buffer.h +++ b/webrtc/base/buffer.h @@ -219,7 +219,7 @@ class BufferT { void AppendData(const U* data, size_t size) { RTC_DCHECK(IsConsistent()); const size_t new_size = size_ + size; - EnsureCapacity(new_size); + EnsureCapacityWithHeadroom(new_size, true); static_assert(sizeof(T) == sizeof(U), ""); std::memcpy(data_.get() + size_, data, size * sizeof(U)); size_ = new_size; @@ -272,7 +272,7 @@ class BufferT { // the existing contents will be kept and the new space will be // uninitialized. void SetSize(size_t size) { - EnsureCapacity(size); + EnsureCapacityWithHeadroom(size, true); size_ = size; } @@ -280,14 +280,9 @@ class BufferT { // 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 new_data(new T[capacity]); - std::memcpy(new_data.get(), data_.get(), size_ * sizeof(T)); - data_ = std::move(new_data); - capacity_ = capacity; - RTC_DCHECK(IsConsistent()); + // Don't allocate extra headroom, since the user is asking for a specific + // capacity. + EnsureCapacityWithHeadroom(capacity, false); } // Resets the buffer to zero size without altering capacity. Works even if the @@ -306,6 +301,27 @@ class BufferT { } private: + void EnsureCapacityWithHeadroom(size_t capacity, bool extra_headroom) { + RTC_DCHECK(IsConsistent()); + if (capacity <= capacity_) + return; + + // If the caller asks for extra headroom, ensure that the new capacity is + // >= 1.5 times the old capacity. Any constant > 1 is sufficient to prevent + // quadratic behavior; as to why we pick 1.5 in particular, see + // https://github.com/facebook/folly/blob/master/folly/docs/FBVector.md and + // http://www.gahcep.com/cpp-internals-stl-vector-part-1/. + const size_t new_capacity = + extra_headroom ? std::max(capacity, capacity_ + capacity_ / 2) + : capacity; + + std::unique_ptr new_data(new T[new_capacity]); + std::memcpy(new_data.get(), data_.get(), size_ * sizeof(T)); + data_ = std::move(new_data); + capacity_ = new_capacity; + RTC_DCHECK(IsConsistent()); + } + // 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 diff --git a/webrtc/base/buffer_unittest.cc b/webrtc/base/buffer_unittest.cc index e9a853c615..bd095a6b09 100644 --- a/webrtc/base/buffer_unittest.cc +++ b/webrtc/base/buffer_unittest.cc @@ -68,7 +68,7 @@ TEST(BufferTest, TestSetData) { Buffer buf(kTestData + 4, 7); buf.SetData(kTestData, 9); EXPECT_EQ(buf.size(), 9u); - EXPECT_EQ(buf.capacity(), 9u); + EXPECT_EQ(buf.capacity(), 7u * 3 / 2); EXPECT_EQ(0, memcmp(buf.data(), kTestData, 9)); } @@ -95,7 +95,7 @@ TEST(BufferTest, TestSetSizeLarger) { EXPECT_EQ(buf.capacity(), 15u); buf.SetSize(20); EXPECT_EQ(buf.size(), 20u); - EXPECT_EQ(buf.capacity(), 20u); // Has grown. + EXPECT_EQ(buf.capacity(), 15u * 3 / 2); // Has grown. EXPECT_EQ(0, memcmp(buf.data(), kTestData, 15)); }