Factor out (and optimize) MessageInTransit.

This is mainly so that MessageInTransit has room for a header (for use in other
code), but while at it I optimized it a bit. The 10- and 100-byte
SystemPerftest.MessagePipe_WriteAndRead performance tests go from ~2.3 million
iterations/second to ~2.6; in perf, |free()| is no longer the top hotspot in
SystemPerftest.MessagePipe_WriteAndRead (all sizes) -- instead, |memcpy()| is,
as you'd hope.

R=darin

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

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

4 files changed, 108 insertions, 17 deletions

diff --git a/mojo/system/message_in_transit.cc b/mojo/system/message_in_transit.cc
new file mode 100644
index 0000000..5a39ded
--- /dev/null
+++ b/mojo/system/message_in_transit.cc
@@ -0,0 +1,38 @@
+// 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 "mojo/system/message_in_transit.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+#include <new>
+
+#include "base/basictypes.h"
+#include "mojo/system/limits.h"
+
+namespace mojo {
+namespace system {
+
+// Avoid dangerous situations, but making sure that the size of the "header" +
+// the size of the data fits into a 31-bit number.
+COMPILE_ASSERT(static_cast<uint64_t>(sizeof(MessageInTransit)) +
+                   kMaxMessageNumBytes <= 0x7fffffff,
+               kMaxMessageNumBytes_too_big);
+
+// static
+MessageInTransit* MessageInTransit::Create(const void* bytes,
+                                           uint32_t num_bytes) {
+  // Store the data immediately after the "header", so allocate the needed
+  // space.
+  char* buffer = static_cast<char*>(
+      malloc(sizeof(MessageInTransit) + static_cast<size_t>(num_bytes)));
+  // And do a placement new.
+  MessageInTransit* rv = new (buffer) MessageInTransit(num_bytes);
+  memcpy(buffer + sizeof(MessageInTransit), bytes, num_bytes);
+  return rv;
+}
+
+}  // namespace system
+}  // namespace mojo
diff --git a/mojo/system/message_in_transit.h b/mojo/system/message_in_transit.h
new file mode 100644
index 0000000..be37b98
--- /dev/null
+++ b/mojo/system/message_in_transit.h
@@ -0,0 +1,61 @@
+// 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.
+
+#ifndef MOJO_SYSTEM_MESSAGE_IN_TRANSIT_H_
+#define MOJO_SYSTEM_MESSAGE_IN_TRANSIT_H_
+
+#include <stdint.h>
+#include <stdlib.h>  // For |free()|.
+
+#include "base/basictypes.h"
+
+namespace mojo {
+namespace system {
+
+// This class is used to represent data in transit. It is thread-unsafe.
+// Note: This class is POD.
+class MessageInTransit {
+ public:
+  // Creates a |MessageInTransit| with the data given by |bytes|/|num_bytes|.
+  static MessageInTransit* Create(const void* bytes, uint32_t num_bytes);
+
+  // Destroys a |MessageInTransit| created using |Create()|.
+  inline void Destroy() {
+    // No need to call the destructor, since we're POD.
+    free(this);
+  }
+
+  // Gets the size of the data (in number of bytes).
+  uint32_t size() const {
+    return size_;
+  }
+
+  // Gets the data (of size |size()| bytes).
+  const void* data() const {
+    return reinterpret_cast<const char*>(this) + sizeof(*this);
+  }
+
+  // TODO(vtl): Add whatever's necessary to transport handles.
+
+ private:
+  explicit MessageInTransit(uint32_t size)
+      : size_(size), reserved_(0), user_1_(0), user_2_(0) {}
+
+  // "Header" for the data.
+  uint32_t size_;
+  uint32_t reserved_;
+  uint32_t user_1_;
+  uint32_t user_2_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageInTransit);
+};
+
+// The size of |MessageInTransit| must be appropriate to maintain alignment of
+// the following data.
+COMPILE_ASSERT(sizeof(MessageInTransit) == 16, MessageInTransit_has_wrong_size);
+
+}  // namespace system
+}  // namespace mojo
+
+#endif  // MOJO_SYSTEM_MESSAGE_IN_TRANSIT_H_
diff --git a/mojo/system/message_pipe.cc b/mojo/system/message_pipe.cc
index 6aebeb3..21ad958 100644
--- a/mojo/system/message_pipe.cc
+++ b/mojo/system/message_pipe.cc
@@ -8,6 +8,7 @@
 #include "base/stl_util.h"
 #include "mojo/system/limits.h"
 #include "mojo/system/memory.h"
+#include "mojo/system/message_in_transit.h"
 
 namespace mojo {
 namespace system {
@@ -108,7 +109,7 @@ MojoResult MessagePipe::WriteMessage(
 
   // TODO(vtl): Eventually (with C++11), this should be an |emplace_back()|.
   message_queues_[destination_port].push_back(
-      new MessageInTransit(bytes, num_bytes));
+      MessageInTransit::Create(bytes, num_bytes));
   // TODO(vtl): Support sending handles.
 
   // The other (destination) port was empty and now isn't, so it should now be
@@ -128,11 +129,11 @@ MojoResult MessagePipe::ReadMessage(unsigned port,
                                     MojoReadMessageFlags flags) {
   DCHECK(port == 0 || port == 1);
 
-  const size_t max_bytes = num_bytes ? *num_bytes : 0;
+  const uint32_t max_bytes = num_bytes ? *num_bytes : 0;
   if (!VerifyUserPointer<void>(bytes, max_bytes))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
-  const size_t max_handles = num_handles ? *num_handles : 0;
+  const uint32_t max_handles = num_handles ? *num_handles : 0;
   if (!VerifyUserPointer<MojoHandle>(handles, max_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
@@ -146,11 +147,10 @@ MojoResult MessagePipe::ReadMessage(unsigned port,
   // and release the lock immediately.
   bool not_enough_space = false;
   MessageInTransit* const message = message_queues_[port].front();
-  const size_t message_size = message->data.size();
   if (num_bytes)
-    *num_bytes = static_cast<uint32_t>(message_size);
-  if (message_size <= max_bytes)
-    memcpy(bytes, message->data.data(), message_size);
+    *num_bytes = message->size();
+  if (message->size() <= max_bytes)
+    memcpy(bytes, message->data(), message->size());
   else
     not_enough_space = true;
 
@@ -160,7 +160,7 @@ MojoResult MessagePipe::ReadMessage(unsigned port,
 
   if (!not_enough_space || (flags & MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)) {
     message_queues_[port].pop_front();
-    delete message;
+    message->Destroy();
 
     // Now it's empty, thus no longer readable.
     if (message_queues_[port].empty()) {
diff --git a/mojo/system/message_pipe.h b/mojo/system/message_pipe.h
index c02f29e..d8dc640 100644
--- a/mojo/system/message_pipe.h
+++ b/mojo/system/message_pipe.h
@@ -6,7 +6,6 @@
 #define MOJO_SYSTEM_MESSAGE_PIPE_H_
 
 #include <list>
-#include <string>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
@@ -18,6 +17,7 @@
 namespace mojo {
 namespace system {
 
+class MessageInTransit;
 class Waiter;
 
 // |MessagePipe| is the secondary object implementing a message pipe (see the
@@ -46,14 +46,6 @@ class MessagePipe : public base::RefCountedThreadSafe<MessagePipe> {
   void RemoveWaiter(unsigned port, Waiter* waiter);
 
  private:
-  struct MessageInTransit {
-    MessageInTransit(const void* bytes, uint32_t num_bytes)
-        : data(static_cast<const char*>(bytes), num_bytes) {}
-
-    // TODO(vtl): Replace with something more efficient.
-    std::string data;
-  };
-
   friend class base::RefCountedThreadSafe<MessagePipe>;
   virtual ~MessagePipe();