[BlobAsync] Faster shortcuttin, make renderer controller leaky & alive.

content/child/blob_storage/blob_transport_controller.cc

 // Copyright 2015 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/blob_storage/blob_transport_controller.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/shared_memory.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "content/child/blob_storage/blob_consolidation.h"
 #include "content/child/child_process.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/common/fileapi/webblob_messages.h"
 #include "ipc/ipc_sender.h"
 #include "storage/common/blob_storage/blob_item_bytes_request.h"
 #include "storage/common/blob_storage/blob_item_bytes_response.h"
 #include "storage/common/data_element.h"
 
 using base::SharedMemory;
 using base::SharedMemoryHandle;
 using storage::BlobItemBytesRequest;
 using storage::BlobItemBytesResponse;
 using storage::IPCBlobItemRequestStrategy;
 using storage::DataElement;
+using storage::kBlobStorageIPCThresholdBytes;
 
 namespace content {
 
 using storage::IPCBlobCreationCancelCode;
 
 using ConsolidatedItem = BlobConsolidation::ConsolidatedItem;
 using ReadStatus = BlobConsolidation::ReadStatus;
 
 namespace {
-const size_t kLargeThresholdBytes = 250 * 1024;
-static base::LazyInstance<BlobTransportController> g_controller =
+static base::LazyInstance<BlobTransportController>::Leaky g_controller =
     LAZY_INSTANCE_INITIALIZER;
 
 // This keeps the process alive while blobs are being transferred.
 void IncChildProcessRefCount() {
   ChildProcess::current()->AddRefProcess();
 }
 
 void DecChildProcessRefCount() {
   ChildProcess::current()->ReleaseProcess();
 }
 }  // namespace
 
 BlobTransportController* BlobTransportController::GetInstance() {
   return g_controller.Pointer();
 }
 
-BlobTransportController::~BlobTransportController() {}
-
+// static
 void BlobTransportController::InitiateBlobTransfer(
     const std::string& uuid,
     scoped_ptr<BlobConsolidation> consolidation,
-    IPC::Sender* sender,
+    scoped_refptr<ThreadSafeSender> sender,
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_runner,
     scoped_refptr<base::SingleThreadTaskRunner> main_runner) {
+  if (main_runner->BelongsToCurrentThread()) {
+    IncChildProcessRefCount();

[michaeln, 2016/04/05 00:18:04]

having to do this immediately makes sense

+  } else {
+    main_runner->PostTask(FROM_HERE, base::Bind(&IncChildProcessRefCount));
+  }
+
+  // If we fit in IPC, then shortcut our process by sending the descriptions
+  // right away. We schedule our task on the IO thread first as an extra
+  // precaution that we will store the consolidation object before we get any
+  // response from the browser.
   BlobConsolidation* consolidation_ptr = consolidation.get();
-  if (blob_storage_.empty()) {
-    main_thread_runner_ = std::move(main_runner);
-    main_thread_runner_->PostTask(FROM_HERE,
-                                  base::Bind(&IncChildProcessRefCount));
+  IPC::Sender* sender_ptr = sender.get();
+  bool presend_descriptions =
+      consolidation->total_memory() <= kBlobStorageIPCThresholdBytes;
+  io_runner->PostTask(
+      FROM_HERE,
+      base::Bind(&BlobTransportController::InitiateBlobTransferOnIOThread,
+                 base::Unretained(BlobTransportController::GetInstance()), uuid,
+                 base::Passed(std::move(consolidation)),
+                 base::Passed(std::move(sender)), presend_descriptions,
+                 base::Passed(std::move(main_runner))));
+  if (presend_descriptions) {

[michaeln, 2016/04/05 00:18:04]

Are you sure the presending is necessary? If it's necessary for smaller blobs,
does that mean larger blobs are still bugged?

Oh, I see, this is not about correctness, this is to negate a perf regression.
Do you know if it actually does negate the perf regression?

Hmmm, I think if ThreadSafeSender were smart enough to put the message on the
wire immediately when invoked from the IO thread instead of posting an
intermediary task to do so, this presending would not make a difference. But I
don't think ThreadSafeSender/SyncMessageFilter has those smarts, and giving it
those smarts could have other consequences as a result changing the other
message ordering???

[dmurph, 2016/04/05 00:32:04]

I don't know if it does, but I strongly suspect this was the reason. Can I leave
this in there (it's a valid optimization nonetheless), and then make a new patch
for syncmessagefilter?

+    std::vector<storage::DataElement> descriptions;
+    BlobTransportController::GetDescriptions(
+        consolidation_ptr, kBlobStorageIPCThresholdBytes, &descriptions);
+    sender_ptr->Send(new BlobStorageMsg_StartBuildingBlob(uuid, descriptions));
   }
-  blob_storage_[uuid] = std::move(consolidation);
-  std::vector<storage::DataElement> descriptions;
-  GetDescriptions(consolidation_ptr, kLargeThresholdBytes, &descriptions);
-  sender->Send(new BlobStorageMsg_StartBuildingBlob(uuid, descriptions));
 }
 
 void BlobTransportController::OnMemoryRequest(
     const std::string& uuid,
     const std::vector<storage::BlobItemBytesRequest>& requests,
     std::vector<base::SharedMemoryHandle>* memory_handles,
     const std::vector<IPC::PlatformFileForTransit>& file_handles,
     IPC::Sender* sender) {
   std::vector<storage::BlobItemBytesResponse> responses;
   ResponsesStatus status =
@@ skipping 24 matching lines @@
     storage::IPCBlobCreationCancelCode code) {
   DVLOG(1) << "Received blob cancel for blob " << uuid
            << " with code: " << static_cast<int>(code);
   ReleaseBlobConsolidation(uuid);
 }
 
 void BlobTransportController::OnDone(const std::string& uuid) {
   ReleaseBlobConsolidation(uuid);
 }
 
-void BlobTransportController::ClearForTesting() {
-  if (!blob_storage_.empty() && main_thread_runner_) {
-    main_thread_runner_->PostTask(FROM_HERE,
-                                  base::Bind(&DecChildProcessRefCount));
-  }
-  blob_storage_.clear();
-}
-
-BlobTransportController::BlobTransportController() {}
-
+// static
 void BlobTransportController::GetDescriptions(
     BlobConsolidation* consolidation,
     size_t max_data_population,
     std::vector<storage::DataElement>* out) {
   DCHECK(out->empty());
   DCHECK(consolidation);
   const auto& consolidated_items = consolidation->consolidated_items();
 
   size_t current_memory_population = 0;
   size_t current_item = 0;
@@ skipping 33 matching lines @@
       }
       case DataElement::TYPE_DISK_CACHE_ENTRY:
       case DataElement::TYPE_BYTES_DESCRIPTION:
       case DataElement::TYPE_UNKNOWN:
         NOTREACHED();
     }
     ++current_item;
   }
 }
 
+BlobTransportController::BlobTransportController() {}
+
+BlobTransportController::~BlobTransportController() {}
+
+void BlobTransportController::ClearForTesting() {
+  if (!blob_storage_.empty() && main_thread_runner_) {
+    main_thread_runner_->PostTask(FROM_HERE,
+                                  base::Bind(&DecChildProcessRefCount));
+  }
+  blob_storage_.clear();
+}
+
+void BlobTransportController::InitiateBlobTransferOnIOThread(
+    const std::string& uuid,
+    scoped_ptr<BlobConsolidation> consolidation,
+    scoped_refptr<ThreadSafeSender> sender,
+    bool sent_descriptions,
+    scoped_refptr<base::SingleThreadTaskRunner> main_runner) {
+  if (!main_thread_runner_.get()) {
+    main_thread_runner_ = std::move(main_runner);
+  }
+  BlobConsolidation* consolidation_ptr = consolidation.get();
+  blob_storage_[uuid] = std::move(consolidation);
+  if (!sent_descriptions) {
+    std::vector<storage::DataElement> descriptions;
+    GetDescriptions(consolidation_ptr, kBlobStorageIPCThresholdBytes,
+                    &descriptions);
+    sender->Send(new BlobStorageMsg_StartBuildingBlob(uuid, descriptions));
+  }
+}
+
 BlobTransportController::ResponsesStatus BlobTransportController::GetResponses(
     const std::string& uuid,
     const std::vector<BlobItemBytesRequest>& requests,
     std::vector<SharedMemoryHandle>* memory_handles,
     const std::vector<IPC::PlatformFileForTransit>& file_handles,
     std::vector<BlobItemBytesResponse>* out) {
   DCHECK(out->empty());
   auto it = blob_storage_.find(uuid);
   if (it == blob_storage_.end())
     return ResponsesStatus::BLOB_NOT_FOUND;
@@ skipping 57 matching lines @@
       case IPCBlobItemRequestStrategy::UNKNOWN:
         NOTREACHED();
         break;
     }
   }
   return ResponsesStatus::SUCCESS;
 }
 
 void BlobTransportController::ReleaseBlobConsolidation(
     const std::string& uuid) {
-  // If we erased something and we're now empty, release the child process
-  // ref count and deref the main thread runner.
-  if (blob_storage_.erase(uuid) && blob_storage_.empty()) {
+  if (blob_storage_.erase(uuid)) {
     main_thread_runner_->PostTask(FROM_HERE,
                                   base::Bind(&DecChildProcessRefCount));
-    main_thread_runner_ = nullptr;
   }
 }
 
 }  // namespace content