Pepper UDP socket: buffer received packets in the plugin process to improve performance.

ppapi/proxy/resource_reply_thread_registrar.cc

Index: ppapi/proxy/resource_reply_thread_registrar.cc
diff --git a/ppapi/proxy/resource_reply_thread_registrar.cc b/ppapi/proxy/resource_reply_thread_registrar.cc
index 13329872aa17c037133ecfd85422e7eca33bc7c2..7e294fe835885ad4ea0596da2e00c5f09c536164 100644
--- a/ppapi/proxy/resource_reply_thread_registrar.cc
+++ b/ppapi/proxy/resource_reply_thread_registrar.cc
@@ -6,6 +6,8 @@
 
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "ipc/ipc_message.h"
+#include "ppapi/proxy/resource_message_params.h"
 #include "ppapi/shared_impl/proxy_lock.h"
 #include "ppapi/shared_impl/tracked_callback.h"
 
@@ -13,8 +15,8 @@ namespace ppapi {
 namespace proxy {
 
 ResourceReplyThreadRegistrar::ResourceReplyThreadRegistrar(
-    scoped_refptr<base::MessageLoopProxy> default_thread)
-    : default_thread_(default_thread) {
+    scoped_refptr<base::MessageLoopProxy> main_thread)
+    : main_thread_(main_thread) {
 }
 
 ResourceReplyThreadRegistrar::~ResourceReplyThreadRegistrar() {
@@ -26,7 +28,7 @@ void ResourceReplyThreadRegistrar::Register(
     scoped_refptr<TrackedCallback> reply_thread_hint) {
   ProxyLock::AssertAcquiredDebugOnly();
 
-  // Use the default thread if |reply_thread_hint| is NULL or blocking.
+  // Use the main thread if |reply_thread_hint| is NULL or blocking.
   if (!reply_thread_hint.get() || reply_thread_hint->is_blocking())
     return;
 
@@ -36,35 +38,48 @@ void ResourceReplyThreadRegistrar::Register(
   {
     base::AutoLock auto_lock(lock_);
 
-    if (reply_thread.get() == default_thread_.get())
+    if (reply_thread.get() == main_thread_.get())
       return;
 
-    map_[resource][sequence_number] = reply_thread;
+    map_[resource].thread_map[sequence_number] = reply_thread;
   }
 }
 
+void ResourceReplyThreadRegistrar::HandleOnIOThread(
+    PP_Resource resource, uint32 nested_msg_type) {
+  base::AutoLock auto_lock(lock_);
+  map_[resource].io_thread_message_types.insert(nested_msg_type);
+}
+
 void ResourceReplyThreadRegistrar::Unregister(PP_Resource resource) {
   base::AutoLock auto_lock(lock_);
   map_.erase(resource);
 }
 
 scoped_refptr<base::MessageLoopProxy>
-ResourceReplyThreadRegistrar::GetTargetThreadAndUnregister(
-    PP_Resource resource,
-    int32_t sequence_number) {
+ResourceReplyThreadRegistrar::GetTargetThread(
+    const ResourceMessageReplyParams& reply_params,
+    const IPC::Message& nested_msg) {
   base::AutoLock auto_lock(lock_);
-  ResourceMap::iterator resource_iter = map_.find(resource);
+  ResourceMap::iterator resource_iter = map_.find(reply_params.pp_resource());
   if (resource_iter == map_.end())
-    return default_thread_;
+    return main_thread_;
 
-  SequenceNumberMap::iterator sequence_number_iter =
-      resource_iter->second.find(sequence_number);
-  if (sequence_number_iter == resource_iter->second.end())
-    return default_thread_;
+  ResourceInfo& info = resource_iter->second;
+  SequenceThreadMap::iterator sequence_thread_iter =
+      info.thread_map.find(reply_params.sequence());
+  if (sequence_thread_iter != info.thread_map.end()) {
+    scoped_refptr<base::MessageLoopProxy> target = sequence_thread_iter->second;
+    info.thread_map.erase(sequence_thread_iter);

[dmichael (off chromium), 2014/09/12 19:49:07]

After this, do you want something like...
if (info.thread_map.empty())
  map_.erase(resource_iter);

To clean the PP_Resource out of map_ if it has no messages registered anymore?
(Note this assumes you move the io thread stuff out to a separate set; otherwise
you can't safely remove PP_Resources from map_)

[yzshen1, 2014/09/15 20:59:39]

When a plugin resource is destroyed, we call Unregister() in its destructor. So
the resource won't stay in map_ forever.
I didn't remove it here because if a resource calls Register(), it is very
likely to call Register() again. By leaving the resource in map_, we save some
insertion/removal.

[dmichael (off chromium), 2014/09/16 17:39:00]

Okay, thanks for explaining.

+    return target;
+  }
+
+  if (info.io_thread_message_types.find(nested_msg.type()) !=

[dmichael (off chromium), 2014/09/12 19:49:06]

nit: could just use io_thread_message_types.count(nested_msg.type())

[yzshen1, 2014/09/15 20:59:39]

Is it because there is a performance difference? (I don't know the answer.) I
thought find() conveys our intention a little more clear, but I am fine changing
it if you prefer the other way.

[dmichael (off chromium), 2014/09/16 17:39:00]

No performance difference; just shorter. I find count() easier to read
personally, just because it's less text for my brain to parse. And using it this
way is fairly idiomatic IMO. But in base/stl_util.h there's also ContainsKey,
which might be most readable of all. It's totally minor; I usually use count(),
but they're all fine to me.

[yzshen1, 2014/09/16 19:09:07]

Done. Changed to count().

+          info.io_thread_message_types.end()) {
+    return scoped_refptr<base::MessageLoopProxy>();
+  }
 
-  scoped_refptr<base::MessageLoopProxy> target = sequence_number_iter->second;
-  resource_iter->second.erase(sequence_number_iter);
-  return target;
+  return main_thread_;
 }
 
 }  // namespace proxy