Modify SpdyHeaderBlock's internals to consolidate header values only on first access.

net/spdy/spdy_header_block.cc

Index: net/spdy/spdy_header_block.cc
diff --git a/net/spdy/spdy_header_block.cc b/net/spdy/spdy_header_block.cc
index 4e36b8927b57529a3c72405c2a95bdb51e01748f..6cbd6ba645c4585a3126099322effb3f9525c968 100644
--- a/net/spdy/spdy_header_block.cc
+++ b/net/spdy/spdy_header_block.cc
@@ -19,6 +19,7 @@
 using base::StringPiece;
 using std::dec;
 using std::hex;
+using std::make_pair;
 using std::max;
 using std::min;
 using std::string;
@@ -30,6 +31,16 @@ namespace {
 const size_t kDefaultStorageBlockSize = 2048;
 
 const char kCookieKey[] = "cookie";
+const char kNullSeparator = 0;
+
+StringPiece SeparatorForKey(StringPiece key) {
+  if (key == kCookieKey) {
+    static StringPiece cookie_separator = "; ";
+    return cookie_separator;
+  } else {
+    return StringPiece(&kNullSeparator, 1);
+  }
+}
 
 }  // namespace
 
@@ -50,24 +61,6 @@ class SpdyHeaderBlock::Storage {
     return StringPiece(arena_.Memdup(s.data(), s.size()), s.size());
   }
 
-  // Given value, a string already in the arena, perform a realloc and append
-  // separator and more to the end of the value's new location. If value is the
-  // most recently added string (via Write), then UnsafeArena will not copy the
-  // existing value but instead will increase the space reserved for value.
-  StringPiece Realloc(StringPiece value,
-                      StringPiece separator,
-                      StringPiece more) {
-    size_t total_length = value.size() + separator.size() + more.size();
-    char* ptr = const_cast<char*>(value.data());
-    ptr = arena_.Realloc(ptr, value.size(), total_length);
-    StringPiece result(ptr, total_length);
-    ptr += value.size();
-    memcpy(ptr, separator.data(), separator.size());
-    ptr += separator.size();
-    memcpy(ptr, more.data(), more.size());
-    return result;
-  }
-
   // If |s| points to the most recent allocation from arena_, the arena will
   // reclaim the memory. Otherwise, this method is a no-op.
   void Rewind(const StringPiece s) {
@@ -76,10 +69,77 @@ class SpdyHeaderBlock::Storage {
 
   void Clear() { arena_.Reset(); }
 
+  // Given a list of fragments and a separator, writes the fragments joined by
+  // the separator to a contiguous region of memory. Returns a StringPiece
+  // pointing to the region of memory.
+  StringPiece WriteFragments(const std::vector<StringPiece>& fragments,
+                             StringPiece separator) {
+    if (fragments.empty()) {
+      return StringPiece();
+    }
+    size_t total_size = separator.size() * (fragments.size() - 1);
+    for (const auto fragment : fragments) {
+      total_size += fragment.size();
+    }
+    char* dst = arena_.Alloc(total_size);
+    size_t written = Join(dst, fragments, separator);
+    DCHECK_EQ(written, total_size);
+    return StringPiece(dst, total_size);
+  }
+
+  size_t bytes_allocated() const { return arena_.status().bytes_allocated(); }
+
  private:
   UnsafeArena arena_;
 };
 
+SpdyHeaderBlock::HeaderValue::HeaderValue(Storage* storage,
+                                          StringPiece key,
+                                          StringPiece initial_value)
+    : storage_(storage), fragments_({initial_value}), pair_({key, {}}) {}
+
+SpdyHeaderBlock::HeaderValue::HeaderValue(HeaderValue&& other)
+    : storage_(other.storage_),
+      fragments_(std::move(other.fragments_)),
+      pair_(std::move(other.pair_)) {}
+
+SpdyHeaderBlock::HeaderValue& SpdyHeaderBlock::HeaderValue::operator=(
+    HeaderValue&& other) {
+  storage_ = other.storage_;
+  fragments_ = std::move(other.fragments_);
+  pair_ = std::move(other.pair_);
+  return *this;
+}
+
+SpdyHeaderBlock::HeaderValue::~HeaderValue() {}
+
+StringPiece SpdyHeaderBlock::HeaderValue::ConsolidatedValue() const {
+  if (fragments_.empty()) {
+    return StringPiece();
+  }
+  if (fragments_.size() > 1) {
+    fragments_ = {
+        storage_->WriteFragments(fragments_, SeparatorForKey(pair_.first))};
+  }
+  return fragments_[0];
+}
+
+void SpdyHeaderBlock::HeaderValue::Append(StringPiece fragment) {
+  fragments_.push_back(fragment);
+}
+
+const std::pair<StringPiece, StringPiece>&
+SpdyHeaderBlock::HeaderValue::as_pair() const {
+  pair_.second = ConsolidatedValue();
+  return pair_;
+}
+
+SpdyHeaderBlock::iterator::iterator(MapType::const_iterator it) : it_(it) {}
+
+SpdyHeaderBlock::iterator::iterator(const iterator& other) : it_(other.it_) {}
+
+SpdyHeaderBlock::iterator::~iterator() {}
+
 SpdyHeaderBlock::ValueProxy::ValueProxy(
     SpdyHeaderBlock::MapType* block,
     SpdyHeaderBlock::Storage* storage,
@@ -89,8 +149,7 @@ SpdyHeaderBlock::ValueProxy::ValueProxy(
       storage_(storage),
       lookup_result_(lookup_result),
       key_(key),
-      valid_(true) {
-}
+      valid_(true) {}
 
 SpdyHeaderBlock::ValueProxy::ValueProxy(ValueProxy&& other)
     : block_(other.block_),
@@ -127,10 +186,14 @@ SpdyHeaderBlock::ValueProxy& SpdyHeaderBlock::ValueProxy::operator=(
   if (lookup_result_ == block_->end()) {
     DVLOG(1) << "Inserting: (" << key_ << ", " << value << ")";
     lookup_result_ =
-        block_->insert(std::make_pair(key_, storage_->Write(value))).first;
+        block_
+            ->emplace(make_pair(
+                key_, HeaderValue(storage_, key_, storage_->Write(value))))
+            .first;
   } else {
     DVLOG(1) << "Updating key: " << key_ << " with value: " << value;
-    lookup_result_->second = storage_->Write(value);
+    lookup_result_->second =
+        HeaderValue(storage_, key_, storage_->Write(value));
   }
   return *this;
 }
@@ -139,7 +202,7 @@ string SpdyHeaderBlock::ValueProxy::as_string() const {
   if (lookup_result_ == block_->end()) {
     return "";
   } else {
-    return lookup_result_->second.as_string();
+    return lookup_result_->second.value().as_string();
   }
 }
 
@@ -160,8 +223,8 @@ SpdyHeaderBlock& SpdyHeaderBlock::operator=(SpdyHeaderBlock&& other) {
 
 SpdyHeaderBlock SpdyHeaderBlock::Clone() const {
   SpdyHeaderBlock copy;
-  for (auto iter : *this) {
-    copy.AppendHeader(iter.first, iter.second);
+  for (const auto& p : *this) {
+    copy.AppendHeader(p.first, p.second);
   }
   return copy;
 }
@@ -181,7 +244,7 @@ string SpdyHeaderBlock::DebugString() const {
   string output = "\n{\n";
   for (auto it = begin(); it != end(); ++it) {
     output +=
-        "  " + it->first.as_string() + ":" + it->second.as_string() + "\n";
+        "  " + it->first.as_string() + " " + it->second.as_string() + "\n";
   }
   output.append("}\n");
   return output;
@@ -192,8 +255,7 @@ void SpdyHeaderBlock::clear() {
   storage_.reset();
 }
 
-void SpdyHeaderBlock::insert(
-    const SpdyHeaderBlock::MapType::value_type& value) {
+void SpdyHeaderBlock::insert(const SpdyHeaderBlock::value_type& value) {
   // TODO(birenroy): Write new value in place of old value, if it fits.
   auto iter = block_.find(value.first);
   if (iter == block_.end()) {
@@ -202,7 +264,9 @@ void SpdyHeaderBlock::insert(
   } else {
     DVLOG(1) << "Updating key: " << iter->first
              << " with value: " << value.second;
-    iter->second = GetStorage()->Write(value.second);
+    auto storage = GetStorage();
+    iter->second =
+        HeaderValue(storage, iter->first, storage->Write(value.second));
   }
 }
 
@@ -232,16 +296,15 @@ void SpdyHeaderBlock::AppendValueOrAddHeader(const StringPiece key,
     return;
   }
   DVLOG(1) << "Updating key: " << iter->first << "; appending value: " << value;
-  StringPiece separator("", 1);
-  if (key == kCookieKey) {
-    separator = "; ";
-  }
-  iter->second = GetStorage()->Realloc(iter->second, separator, value);
+  iter->second.Append(GetStorage()->Write(value));
 }
 
 void SpdyHeaderBlock::AppendHeader(const StringPiece key,
                                    const StringPiece value) {
-  block_.emplace(GetStorage()->Write(key), GetStorage()->Write(value));
+  auto storage = GetStorage();
+  auto backed_key = storage->Write(key);
+  block_.emplace(make_pair(
+      backed_key, HeaderValue(storage, backed_key, storage->Write(value))));
 }
 
 SpdyHeaderBlock::Storage* SpdyHeaderBlock::GetStorage() {
@@ -293,4 +356,31 @@ bool SpdyHeaderBlockFromNetLogParam(
   return true;
 }
 
+size_t SpdyHeaderBlock::bytes_allocated() const {
+  if (storage_ == nullptr) {
+    return 0;
+  } else {
+    return storage_->bytes_allocated();
+  }
+}
+
+size_t Join(char* dst,
+            const std::vector<StringPiece>& fragments,
+            StringPiece separator) {
+  if (fragments.empty()) {
+    return 0;
+  }
+  auto original_dst = dst;
+  auto it = fragments.begin();
+  memcpy(dst, it->data(), it->size());
+  dst += it->size();
+  for (++it; it != fragments.end(); ++it) {
+    memcpy(dst, separator.data(), separator.size());
+    dst += separator.size();
+    memcpy(dst, it->data(), it->size());
+    dst += it->size();
+  }
+  return dst - original_dst;
+}
+
 }  // namespace net