Mojo: Optimize VerifyUserPointer().

Gets rid of run-time division. In the new empty read test, goes from ~8.7
million iterations/second to ~9.7 million iterations/second (and in perf makes
VerifyUserPointer() go from ~10% to ~0.5%).

R=darin

Review URL: https://codereview.chromium.org/25323003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226422 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 57 insertions, 18 deletions

diff --git a/mojo/public/tests/system_core_perftest.cc b/mojo/public/tests/system_core_perftest.cc
index 2e9fef2..8ee7ceb 100644
--- a/mojo/public/tests/system_core_perftest.cc
+++ b/mojo/public/tests/system_core_perftest.cc
@@ -45,6 +45,15 @@ class SystemPerftest : public test::TestBase {
     DCHECK_EQ(result, MOJO_RESULT_OK);
   }
 
+  void MessagePipe_EmptyRead() {
+    MojoResult result;
+    result = ReadMessage(h_0_,
+                         NULL, NULL,
+                         NULL, NULL,
+                         MOJO_READ_MESSAGE_FLAG_MAY_DISCARD);
+    DCHECK_EQ(result, MOJO_RESULT_NOT_FOUND);
+  }
+
  protected:
   Handle h_0_;
   Handle h_1_;
@@ -95,5 +104,15 @@ TEST_F(SystemPerftest, MessagePipe_WriteAndRead) {
   CHECK_EQ(Close(h_1_), MOJO_RESULT_OK);
 }
 
+TEST_F(SystemPerftest, MessagePipe_EmptyRead) {
+  CHECK_EQ(CreateMessagePipe(&h_0_, &h_1_), MOJO_RESULT_OK);
+  test::IterateAndReportPerf(
+      "MessagePipe_EmptyRead",
+      base::Bind(&SystemPerftest::MessagePipe_EmptyRead,
+                 base::Unretained(this)));
+  CHECK_EQ(Close(h_0_), MOJO_RESULT_OK);
+  CHECK_EQ(Close(h_1_), MOJO_RESULT_OK);
+}
+
 }  // namespace
 }  // namespace mojo
diff --git a/mojo/system/core_impl.cc b/mojo/system/core_impl.cc
index 00d1107..104a373 100644
--- a/mojo/system/core_impl.cc
+++ b/mojo/system/core_impl.cc
@@ -106,9 +106,9 @@ MojoResult CoreImpl::WaitMany(const MojoHandle* handles,
                               const MojoWaitFlags* flags,
                               uint32_t num_handles,
                               MojoDeadline deadline) {
-  if (!VerifyUserPointer(handles, num_handles, sizeof(handles[0])))
+  if (!VerifyUserPointer<MojoHandle>(handles, num_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
-  if (!VerifyUserPointer(flags, num_handles, sizeof(flags[0])))
+  if (!VerifyUserPointer<MojoWaitFlags>(flags, num_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (num_handles < 1)
     return MOJO_RESULT_INVALID_ARGUMENT;
diff --git a/mojo/system/core_test_base.cc b/mojo/system/core_test_base.cc
index 36241f8..93b11de 100644
--- a/mojo/system/core_test_base.cc
+++ b/mojo/system/core_test_base.cc
@@ -51,9 +51,9 @@ class MockDispatcher : public Dispatcher {
     info_->IncrementWriteMessageCallCount();
     lock().AssertAcquired();
 
-    if (!VerifyUserPointer(bytes, num_bytes, 1))
+    if (!VerifyUserPointer<void>(bytes, num_bytes))
       return MOJO_RESULT_INVALID_ARGUMENT;
-    if (!VerifyUserPointer(handles, num_handles, sizeof(handles[0])))
+    if (!VerifyUserPointer<MojoHandle>(handles, num_handles))
       return MOJO_RESULT_INVALID_ARGUMENT;
 
     return MOJO_RESULT_OK;
@@ -68,10 +68,10 @@ class MockDispatcher : public Dispatcher {
     info_->IncrementReadMessageCallCount();
     lock().AssertAcquired();
 
-    if (num_bytes && !VerifyUserPointer(bytes, *num_bytes, 1))
+    if (num_bytes && !VerifyUserPointer<void>(bytes, *num_bytes))
       return MOJO_RESULT_INVALID_ARGUMENT;
     if (num_handles &&
-        !VerifyUserPointer(handles, *num_handles, sizeof(handles[0])))
+        !VerifyUserPointer<MojoHandle>(handles, *num_handles))
       return MOJO_RESULT_INVALID_ARGUMENT;
 
     return MOJO_RESULT_OK;
diff --git a/mojo/system/memory.cc b/mojo/system/memory.cc
index df658ae..fe7edef 100644
--- a/mojo/system/memory.cc
+++ b/mojo/system/memory.cc
@@ -11,9 +11,9 @@
 namespace mojo {
 namespace system {
 
-bool VerifyUserPointer(const void* pointer, size_t count, size_t size_each) {
-  DCHECK_GT(size_each, 0u);
-  if (count > std::numeric_limits<size_t>::max() / size_each)
+template <size_t size>
+bool VerifyUserPointerForSize(const void* pointer, size_t count) {
+  if (count > std::numeric_limits<size_t>::max() / size)
     return false;
 
   // TODO(vtl): If running in kernel mode, do a full verification. For now, just
@@ -22,5 +22,9 @@ bool VerifyUserPointer(const void* pointer, size_t count, size_t size_each) {
   return count == 0 || !!pointer;
 }
 
+// Explicitly instantiate the sizes we need. Add instantiations as needed.
+template bool VerifyUserPointerForSize<1>(const void*, size_t);
+template bool VerifyUserPointerForSize<4>(const void*, size_t);
+
 }  // namespace system
 }  // namespace mojo
diff --git a/mojo/system/memory.h b/mojo/system/memory.h
index 6a1dafc..cca3aca 100644
--- a/mojo/system/memory.h
+++ b/mojo/system/memory.h
@@ -10,16 +10,32 @@
 namespace mojo {
 namespace system {
 
-// Verify that |count * size_each| bytes can be read from the user |pointer|
-// insofar as possible/necessary. |count| and |size_each| are specified
-// separately instead of a single size, since |count * size_each| may overflow a
-// |size_t|. |count| may be zero but |size_each| must be nonzero.
+// This is just forward-declared, with the definition and explicit
+// instantiations in the .cc file. This is used by |VerifyUserPointer<T>()|
+// below, and you should use that instead.
+template <size_t size>
+bool VerifyUserPointerForSize(const void* pointer, size_t count);
+
+// Verify that |count * sizeof(T)| bytes can be read from the user |pointer|
+// insofar as possible/necessary (note: this is done carefully since |count *
+// sizeof(T)| may overflow a |size_t|. |count| may be zero. If |T| is |void|,
+// then the size of each element is taken to be a single byte.
 //
 // For example, if running in kernel mode, this should be a full verification
 // that the given memory is owned and readable by the user process. In user
 // mode, if crashes are acceptable, this may do nothing at all (and always
 // return true).
-bool VerifyUserPointer(const void* pointer, size_t count, size_t size_each);
+template <typename T>
+bool VerifyUserPointer(const T* pointer, size_t count) {
+  return VerifyUserPointerForSize<sizeof(T)>(pointer, count);
+}
+
+// Special-case |T| equals |void| so that the size is in bytes, as indicated
+// above.
+template <>
+inline bool VerifyUserPointer<void>(const void* pointer, size_t count) {
+  return VerifyUserPointerForSize<1>(pointer, count);
+}
 
 }  // namespace system
 }  // namespace mojo
diff --git a/mojo/system/message_pipe.cc b/mojo/system/message_pipe.cc
index 8461705..6aebeb3 100644
--- a/mojo/system/message_pipe.cc
+++ b/mojo/system/message_pipe.cc
@@ -80,12 +80,12 @@ MojoResult MessagePipe::WriteMessage(
 
   unsigned destination_port = DestinationPortFromSourcePort(port);
 
-  if (!VerifyUserPointer(bytes, num_bytes, 1))
+  if (!VerifyUserPointer<void>(bytes, num_bytes))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (num_bytes > kMaxMessageNumBytes)
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
 
-  if (!VerifyUserPointer(handles, num_handles, sizeof(handles[0])))
+  if (!VerifyUserPointer<MojoHandle>(handles, num_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (num_handles > kMaxMessageNumHandles)
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
@@ -129,11 +129,11 @@ MojoResult MessagePipe::ReadMessage(unsigned port,
   DCHECK(port == 0 || port == 1);
 
   const size_t max_bytes = num_bytes ? *num_bytes : 0;
-  if (!VerifyUserPointer(bytes, max_bytes, 1))
+  if (!VerifyUserPointer<void>(bytes, max_bytes))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   const size_t max_handles = num_handles ? *num_handles : 0;
-  if (!VerifyUserPointer(handles, max_handles, sizeof(handles[0])))
+  if (!VerifyUserPointer<MojoHandle>(handles, max_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   base::AutoLock locker(lock_);