[Storage] Consoliation of BlobItems with small size

content/child/webblobregistry_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/webblobregistry_impl.h"
 
 #include "base/files/file_path.h"
 #include "base/guid.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory.h"
@@ skipping 25 matching lines @@
     : sender_(sender) {
 }
 
 WebBlobRegistryImpl::~WebBlobRegistryImpl() {
 }
 
 void WebBlobRegistryImpl::registerBlobData(
     const blink::WebString& uuid, const blink::WebBlobData& data) {
   const std::string uuid_str(uuid.utf8());
 
+  std::vector<char> buf_;

[michaeln, 2015/01/21 02:16:48]

nit: nix trailing underbar

[dmurph, 2015/01/21 23:57:42]

Done.

+  buf_.reserve(kLargeThresholdBytes);
+  size_t consolidating_items_size_bytes;

[michaeln, 2015/01/21 02:16:48]

do we need this local? what about buf.size()?

[dmurph, 2015/01/21 23:57:42]

Ah, I was getting confused with arrays and vectors here.

+
   sender_->Send(new BlobHostMsg_StartBuilding(uuid_str));
   size_t i = 0;
   WebBlobData::Item data_item;
   while (data.itemAt(i++, data_item)) {

[michaeln, 2015/01/21 02:16:48]

in each case below, you could avoid flushing the buffer if length is zero. maybe
have a test like that up top here to continue the loop for empty items

[dmurph, 2015/01/21 23:57:42]

Done.

+    if (data_item.type != WebBlobData::Item::TypeData &&
+        consolidating_items_size_bytes != 0) {
+      SendAndClearConsolidatingBuffer(uuid_str, &buf_,
+                                      &consolidating_items_size_bytes);
+    }
     switch (data_item.type) {
       case WebBlobData::Item::TypeData: {
         // WebBlobData does not allow partial data items.
         DCHECK(!data_item.offset && data_item.length == -1);
-        SendDataForBlob(uuid_str, data_item.data);

[michaeln, 2015/01/21 02:16:47]

might be nice to bury the new buffering logic inside of the BufferDataForBlob()
helper so the registerBlobData() remains more easily read

+        if (consolidating_items_size_bytes + data_item.length >=
+            kLargeThresholdBytes) {
+          SendAndClearConsolidatingBuffer(uuid_str, &buf_,

[michaeln, 2015/01/21 02:16:48]

i think this can get called even if consolidating_items_size_bytes is zero

[dmurph, 2015/01/21 23:57:42]

Done.

+                                          &consolidating_items_size_bytes);
+        }
+        if (data_item.length >= static_cast<long long>(kLargeThresholdBytes)) {
+          SendOversizedDataForBlob(uuid_str, data_item.data);
+        } else {
+          AddItemToBuffer(data_item.data, &buf_,
+                          &consolidating_items_size_bytes);
+        }
         break;
       }
       case WebBlobData::Item::TypeFile:
         if (data_item.length) {
           storage::BlobData::Item item;
           item.SetToFilePathRange(
               base::FilePath::FromUTF16Unsafe(data_item.filePath),
               static_cast<uint64>(data_item.offset),
               static_cast<uint64>(data_item.length),
               base::Time::FromDoubleT(data_item.expectedModificationTime));
@@ skipping 23 matching lines @@
               static_cast<uint64>(data_item.length),
               base::Time::FromDoubleT(data_item.expectedModificationTime));
           sender_->Send(
               new BlobHostMsg_AppendBlobDataItem(uuid_str, item));
         }
         break;
       default:
         NOTREACHED();
     }
   }
+  if (consolidating_items_size_bytes != 0) {
+    SendAndClearConsolidatingBuffer(uuid_str, &buf_,
+                                    &consolidating_items_size_bytes);
+  }
   sender_->Send(new BlobHostMsg_FinishBuilding(
       uuid_str, data.contentType().utf8().data()));
 }
 
 void WebBlobRegistryImpl::addBlobDataRef(const WebString& uuid) {
   sender_->Send(new BlobHostMsg_IncrementRefCount(uuid.utf8()));
 }
 
 void WebBlobRegistryImpl::removeBlobDataRef(const WebString& uuid) {
   sender_->Send(new BlobHostMsg_DecrementRefCount(uuid.utf8()));
 }
 
 void WebBlobRegistryImpl::registerPublicBlobURL(
     const WebURL& url, const WebString& uuid) {
   sender_->Send(new BlobHostMsg_RegisterPublicURL(url, uuid.utf8()));
 }
 
 void WebBlobRegistryImpl::revokePublicBlobURL(const WebURL& url) {
   sender_->Send(new BlobHostMsg_RevokePublicURL(url));
 }
 
-void WebBlobRegistryImpl::SendDataForBlob(const std::string& uuid_str,
-                                          const WebThreadSafeData& data) {
+void WebBlobRegistryImpl::SendAndClearConsolidatingBuffer(

[michaeln, 2015/01/21 02:16:47]

having a test for if (buffer->size()) return in here might help readabilty
elsewhere and avoid sending 0 len items

naming nit: this is a FlushDataBuffer kind of operation

[dmurph, 2015/01/21 23:57:42]

Done.

+    const std::string& uuid_str,
+    std::vector<char>* consolidating_buffer,
+    size_t* consolidating_items_size_bytes) const {
+  DCHECK_NE(*consolidating_items_size_bytes, 0ul);
+  DCHECK_LT(*consolidating_items_size_bytes, kLargeThresholdBytes);
+  storage::BlobData::Item item;
+  item.SetToBytes(consolidating_buffer->data(),
+                  *consolidating_items_size_bytes);
+  sender_->Send(new BlobHostMsg_AppendBlobDataItem(uuid_str, item));
+  *consolidating_items_size_bytes = 0;
+  consolidating_buffer->clear();
+}
 
-  if (data.size() == 0)
+void WebBlobRegistryImpl::AddItemToBuffer(
+    const blink::WebThreadSafeData& data,
+    std::vector<char>* consolidating_buffer,
+    size_t* consolidating_items_size_bytes) const {
+  if (data.size() == 0) {
     return;
-  if (data.size() < kLargeThresholdBytes) {
-    storage::BlobData::Item item;
-    item.SetToBytes(data.data(), data.size());
-    sender_->Send(new BlobHostMsg_AppendBlobDataItem(uuid_str, item));
-  } else {
-    // We handle larger amounts of data via SharedMemory instead of
-    // writing it directly to the IPC channel.
-    size_t shared_memory_size = std::min(
-        data.size(), kMaxSharedMemoryBytes);
-    scoped_ptr<base::SharedMemory> shared_memory(
-        ChildThread::AllocateSharedMemory(shared_memory_size,
-                                          sender_.get()));
-    CHECK(shared_memory.get());
-    if (!shared_memory->Map(shared_memory_size))
-      CHECK(false);
+  }
+  DCHECK_LT(*consolidating_items_size_bytes + data.size(),
+            kLargeThresholdBytes);
+  consolidating_buffer->insert(consolidating_buffer->end(), data.data(),
+                               data.data() + data.size());
+  *consolidating_items_size_bytes += data.size();
+}
 
-    size_t data_size = data.size();
-    const char* data_ptr = data.data();
-    while (data_size) {
-      size_t chunk_size = std::min(data_size, shared_memory_size);
-      memcpy(shared_memory->memory(), data_ptr, chunk_size);
-      sender_->Send(new BlobHostMsg_SyncAppendSharedMemory(
-          uuid_str, shared_memory->handle(), chunk_size));
-      data_size -= chunk_size;
-      data_ptr += chunk_size;
-    }
+void WebBlobRegistryImpl::SendOversizedDataForBlob(
+    const std::string& uuid_str,
+    const WebThreadSafeData& data) {
+  DCHECK_GE(data.size(), kLargeThresholdBytes);
+  // We handle larger amounts of data via SharedMemory instead of
+  // writing it directly to the IPC channel.
+  size_t shared_memory_size = std::min(data.size(), kMaxSharedMemoryBytes);
+  scoped_ptr<base::SharedMemory> shared_memory(
+      ChildThread::AllocateSharedMemory(shared_memory_size, sender_.get()));
+  CHECK(shared_memory.get());
+  if (!shared_memory->Map(shared_memory_size))
+    CHECK(false);
+
+  size_t data_size = data.size();
+  const char* data_ptr = data.data();
+  while (data_size) {
+    size_t chunk_size = std::min(data_size, shared_memory_size);
+    memcpy(shared_memory->memory(), data_ptr, chunk_size);
+    sender_->Send(new BlobHostMsg_SyncAppendSharedMemory(
+        uuid_str, shared_memory->handle(), chunk_size));
+    data_size -= chunk_size;
+    data_ptr += chunk_size;
   }
 }
 
 // ------ streams stuff -----
 
 void WebBlobRegistryImpl::registerStreamURL(
     const WebURL& url, const WebString& content_type) {
   DCHECK(ChildThread::current());
   sender_->Send(new StreamHostMsg_StartBuilding(url, content_type.utf8()));
 }
@@ skipping 52 matching lines @@
   DCHECK(ChildThread::current());
   sender_->Send(new StreamHostMsg_AbortBuilding(url));
 }
 
 void WebBlobRegistryImpl::unregisterStreamURL(const WebURL& url) {
   DCHECK(ChildThread::current());
   sender_->Send(new StreamHostMsg_Remove(url));
 }
 
 }  // namespace content