Bug 957828 - Reduce the number of copies involved in sending IPC Messages. r=bsmedberg, a=1.3+
authorBen Turner <bent.mozilla@gmail.com>
Mon, 27 Jan 2014 09:02:02 -0500
changeset 176029 3e215a0da7805774bd5856969748bf4764b4f382
parent 176028 1cea3bc6f5a12ed964bd437b8d3972bc4ed8077c
child 176030 ef4d9be503aaa502194cb7b6f705036d19a0a697
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg, 1
bugs957828
milestone28.0a2
Bug 957828 - Reduce the number of copies involved in sending IPC Messages. r=bsmedberg, a=1.3+
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -3,16 +3,19 @@
  */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/ipc/MessageChannel.h"
 #include "mozilla/ipc/ProtocolUtils.h"
 
+#include "mozilla/Assertions.h"
+#include "mozilla/DebugOnly.h"
+#include "mozilla/Move.h"
 #include "nsDebug.h"
 #include "nsTraceRefcnt.h"
 
 // Undo the damage done by mozzconf.h
 #undef compress
 
 using namespace mozilla;
 using namespace std;
@@ -36,16 +39,158 @@ struct RunnableMethodTraits<mozilla::ipc
 namespace mozilla {
 namespace ipc {
 
 const int32_t MessageChannel::kNoTimeout = INT32_MIN;
 
 // static
 bool MessageChannel::sIsPumpingMessages = false;
 
+enum Direction
+{
+    IN_MESSAGE,
+    OUT_MESSAGE
+};
+
+
+class MessageChannel::InterruptFrame
+{
+private:
+    enum Semantics
+    {
+        INTR_SEMS,
+        SYNC_SEMS,
+        ASYNC_SEMS
+    };
+
+public:
+    InterruptFrame(Direction direction, const Message* msg)
+      : mMessageName(strdup(msg->name())),
+        mMessageRoutingId(msg->routing_id()),
+        mMesageSemantics(msg->is_interrupt() ? INTR_SEMS :
+                          msg->is_sync() ? SYNC_SEMS :
+                          ASYNC_SEMS),
+        mDirection(direction),
+        mMoved(false)
+    {
+        MOZ_ASSERT(mMessageName);
+    }
+
+    InterruptFrame(InterruptFrame&& aOther)
+    {
+        MOZ_ASSERT(aOther.mMessageName);
+        mMessageName = aOther.mMessageName;
+        aOther.mMessageName = nullptr;
+        aOther.mMoved = true;
+
+        mMessageRoutingId = aOther.mMessageRoutingId;
+        mMesageSemantics = aOther.mMesageSemantics;
+        mDirection = aOther.mDirection;
+    }
+
+    ~InterruptFrame()
+    {
+        MOZ_ASSERT_IF(!mMessageName, mMoved);
+
+        if (mMessageName)
+            free(const_cast<char*>(mMessageName));
+    }
+
+    InterruptFrame& operator=(InterruptFrame&& aOther)
+    {
+        MOZ_ASSERT(&aOther != this);
+        this->~InterruptFrame();
+        new (this) InterruptFrame(mozilla::Move(aOther));
+        return *this;
+    }
+
+    bool IsInterruptIncall() const
+    {
+        return INTR_SEMS == mMesageSemantics && IN_MESSAGE == mDirection;
+    }
+
+    bool IsInterruptOutcall() const
+    {
+        return INTR_SEMS == mMesageSemantics && OUT_MESSAGE == mDirection;
+    }
+
+    void Describe(int32_t* id, const char** dir, const char** sems,
+                  const char** name) const
+    {
+        *id = mMessageRoutingId;
+        *dir = (IN_MESSAGE == mDirection) ? "in" : "out";
+        *sems = (INTR_SEMS == mMesageSemantics) ? "intr" :
+                (SYNC_SEMS == mMesageSemantics) ? "sync" :
+                "async";
+        *name = mMessageName;
+    }
+
+private:
+    const char* mMessageName;
+    int32_t mMessageRoutingId;
+    Semantics mMesageSemantics;
+    Direction mDirection;
+    DebugOnly<bool> mMoved;
+
+    // Disable harmful methods.
+    InterruptFrame(const InterruptFrame& aOther) MOZ_DELETE;
+    InterruptFrame& operator=(const InterruptFrame&) MOZ_DELETE;
+};
+
+class MOZ_STACK_CLASS MessageChannel::CxxStackFrame
+{
+public:
+    CxxStackFrame(MessageChannel& that, Direction direction, const Message* msg)
+      : mThat(that)
+    {
+        mThat.AssertWorkerThread();
+
+        if (mThat.mCxxStackFrames.empty())
+            mThat.EnteredCxxStack();
+
+        mThat.mCxxStackFrames.append(InterruptFrame(direction, msg));
+
+        const InterruptFrame& frame = mThat.mCxxStackFrames.back();
+
+        if (frame.IsInterruptIncall())
+            mThat.EnteredCall();
+
+        mThat.mSawInterruptOutMsg |= frame.IsInterruptOutcall();
+    }
+
+    ~CxxStackFrame() {
+        mThat.AssertWorkerThread();
+
+        MOZ_ASSERT(!mThat.mCxxStackFrames.empty());
+
+        bool exitingCall = mThat.mCxxStackFrames.back().IsInterruptIncall();
+        mThat.mCxxStackFrames.shrinkBy(1);
+
+        bool exitingStack = mThat.mCxxStackFrames.empty();
+
+        // mListener could have gone away if Close() was called while
+        // MessageChannel code was still on the stack
+        if (!mThat.mListener)
+            return;
+
+        if (exitingCall)
+            mThat.ExitedCall();
+
+        if (exitingStack)
+            mThat.ExitedCxxStack();
+    }
+private:
+    MessageChannel& mThat;
+
+    // Disable harmful methods.
+    CxxStackFrame() MOZ_DELETE;
+    CxxStackFrame(const CxxStackFrame&) MOZ_DELETE;
+    CxxStackFrame& operator=(const CxxStackFrame&) MOZ_DELETE;
+};
+
 MessageChannel::MessageChannel(MessageListener *aListener)
   : mListener(aListener->asWeakPtr()),
     mChannelState(ChannelClosed),
     mSide(UnknownSide),
     mLink(nullptr),
     mWorkerLoop(nullptr),
     mChannelErrorTask(nullptr),
     mWorkerLoopID(-1),
@@ -239,18 +384,17 @@ MessageChannel::Echo(Message* aMsg)
 
     mLink->EchoMessage(msg.forget());
     return true;
 }
 
 bool
 MessageChannel::Send(Message* aMsg)
 {
-    Message copy = *aMsg;
-    CxxStackFrame frame(*this, OUT_MESSAGE, &copy);
+    CxxStackFrame frame(*this, OUT_MESSAGE, aMsg);
 
     nsAutoPtr<Message> msg(aMsg);
     AssertWorkerThread();
     mMonitor->AssertNotCurrentThreadOwns();
     if (MSG_ROUTING_NONE == msg->routing_id()) {
         ReportMessageRouteError("MessageChannel::Send");
         return false;
     }
@@ -404,18 +548,17 @@ MessageChannel::Send(Message* aMsg, Mess
     // Sanity checks.
     AssertWorkerThread();
     mMonitor->AssertNotCurrentThreadOwns();
 
 #ifdef OS_WIN
     SyncStackFrame frame(this, false);
 #endif
 
-    Message copy = *aMsg;
-    CxxStackFrame f(*this, OUT_MESSAGE, &copy);
+    CxxStackFrame f(*this, OUT_MESSAGE, aMsg);
 
     MonitorAutoLock lock(*mMonitor);
 
     IPC_ASSERT(aMsg->is_sync(), "can only Send() sync messages here");
     IPC_ASSERT(!DispatchingSyncMessage(), "violation of sync handler invariant");
     IPC_ASSERT(!AwaitingSyncReply(), "nested sync messages are not supported");
 
     AutoEnterPendingReply replies(mPendingSyncReplies);
@@ -432,18 +575,17 @@ MessageChannel::UrgentCall(Message* aMsg
     AssertWorkerThread();
     mMonitor->AssertNotCurrentThreadOwns();
     IPC_ASSERT(mSide == ParentSide, "cannot send urgent requests from child");
 
 #ifdef OS_WIN
     SyncStackFrame frame(this, false);
 #endif
 
-    Message copy = *aMsg;
-    CxxStackFrame f(*this, OUT_MESSAGE, &copy);
+    CxxStackFrame f(*this, OUT_MESSAGE, aMsg);
 
     MonitorAutoLock lock(*mMonitor);
 
     IPC_ASSERT(!AwaitingInterruptReply(), "urgent calls cannot be issued within Interrupt calls");
     IPC_ASSERT(!AwaitingSyncReply(), "urgent calls cannot be issued within sync sends");
 
     AutoEnterRPCTransaction transact(this);
     aMsg->set_transaction_id(mCurrentRPCTransaction);
@@ -462,18 +604,17 @@ MessageChannel::RPCCall(Message* aMsg, M
     AssertWorkerThread();
     mMonitor->AssertNotCurrentThreadOwns();
     IPC_ASSERT(mSide == ChildSide, "cannot send rpc messages from parent");
 
 #ifdef OS_WIN
     SyncStackFrame frame(this, false);
 #endif
 
-    Message copy = *aMsg;
-    CxxStackFrame f(*this, OUT_MESSAGE, &copy);
+    CxxStackFrame f(*this, OUT_MESSAGE, aMsg);
 
     MonitorAutoLock lock(*mMonitor);
 
     AutoEnterRPCTransaction transact(this);
     aMsg->set_transaction_id(mCurrentRPCTransaction);
 
     AutoEnterPendingReply replies(mPendingRPCReplies);
     if (!SendAndWait(aMsg, aReply))
@@ -562,18 +703,17 @@ MessageChannel::InterruptCall(Message* a
     mMonitor->AssertNotCurrentThreadOwns();
 
 #ifdef OS_WIN
     SyncStackFrame frame(this, true);
 #endif
 
     // This must come before MonitorAutoLock, as its destructor acquires the
     // monitor lock.
-    Message copy = *aMsg;
-    CxxStackFrame cxxframe(*this, OUT_MESSAGE, &copy);
+    CxxStackFrame cxxframe(*this, OUT_MESSAGE, aMsg);
 
     MonitorAutoLock lock(*mMonitor);
     if (!Connected()) {
         ReportConnectionError("MessageChannel::Call");
         return false;
     }
 
     // Sanity checks.
@@ -1556,17 +1696,17 @@ void
 MessageChannel::DumpInterruptStack(const char* const pfx) const
 {
     NS_WARN_IF_FALSE(MessageLoop::current() != mWorkerLoop,
                      "The worker thread had better be paused in a debugger!");
 
     printf_stderr("%sMessageChannel 'backtrace':\n", pfx);
 
     // print a python-style backtrace, first frame to last
-    for (uint32_t i = 0; i < mCxxStackFrames.size(); ++i) {
+    for (uint32_t i = 0; i < mCxxStackFrames.length(); ++i) {
         int32_t id;
         const char* dir, *sems, *name;
         mCxxStackFrames[i].Describe(&id, &dir, &sems, &name);
 
         printf_stderr("%s[(%u) %s %s %s(actor=%d) ]\n", pfx,
                       i, dir, sems, name, id);
     }
 }
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -6,26 +6,25 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef ipc_glue_MessageChannel_h
 #define ipc_glue_MessageChannel_h 1
 
 #include "base/basictypes.h"
 #include "base/message_loop.h"
 
+#include "mozilla/Monitor.h"
+#include "mozilla/Vector.h"
 #include "mozilla/WeakPtr.h"
-#include "mozilla/Monitor.h"
 #include "mozilla/ipc/Transport.h"
 #include "MessageLink.h"
 #include "nsAutoPtr.h"
-#include "mozilla/DebugOnly.h"
 
 #include <deque>
 #include <stack>
-#include <vector>
 #include <math.h>
 
 namespace mozilla {
 namespace ipc {
 
 class MessageChannel;
 
 class RefCountedMonitor : public Monitor
@@ -39,16 +38,19 @@ class RefCountedMonitor : public Monitor
 };
 
 class MessageChannel : HasResultCodes
 {
     friend class ProcessLink;
     friend class ThreadLink;
     friend class AutoEnterRPCTransaction;
 
+    class CxxStackFrame;
+    class InterruptFrame;
+
     typedef mozilla::Monitor Monitor;
 
   public:
     static const int32_t kNoTimeout;
 
     typedef IPC::Message Message;
     typedef mozilla::ipc::Transport Transport;
 
@@ -269,88 +271,16 @@ class MessageChannel : HasResultCodes
     void ExitedCall() {
         mListener->OnExitedCall();
     }
 
     MessageListener *Listener() const {
         return mListener.get();
     }
 
-    enum Direction { IN_MESSAGE, OUT_MESSAGE };
-    struct InterruptFrame {
-        InterruptFrame(Direction direction, const Message* msg)
-          : mDirection(direction), mMsg(msg)
-        { }
-
-        bool IsInterruptIncall() const {
-            return mMsg->is_interrupt() && IN_MESSAGE == mDirection;
-        }
-        bool IsInterruptOutcall() const {
-            return mMsg->is_interrupt() && OUT_MESSAGE == mDirection;
-        }
-
-        void Describe(int32_t* id, const char** dir, const char** sems,
-                      const char** name) const
-        {
-            *id = mMsg->routing_id();
-            *dir = (IN_MESSAGE == mDirection) ? "in" : "out";
-            *sems = mMsg->is_interrupt() ? "intr" : mMsg->is_sync() ? "sync" : "async";
-            *name = mMsg->name();
-        }
-
-        Direction mDirection;
-        const Message* mMsg;
-    };
-
-    class MOZ_STACK_CLASS CxxStackFrame
-    {
-      public:
-        CxxStackFrame(MessageChannel& that, Direction direction, const Message* msg)
-          : mThat(that)
-        {
-            mThat.AssertWorkerThread();
-
-            if (mThat.mCxxStackFrames.empty())
-                mThat.EnteredCxxStack();
-
-            mThat.mCxxStackFrames.push_back(InterruptFrame(direction, msg));
-            const InterruptFrame& frame = mThat.mCxxStackFrames.back();
-
-            if (frame.IsInterruptIncall())
-                mThat.EnteredCall();
-
-            mThat.mSawInterruptOutMsg |= frame.IsInterruptOutcall();
-        }
-
-        ~CxxStackFrame() {
-            bool exitingCall = mThat.mCxxStackFrames.back().IsInterruptIncall();
-            mThat.mCxxStackFrames.pop_back();
-            bool exitingStack = mThat.mCxxStackFrames.empty();
-
-            // mListener could have gone away if Close() was called while
-            // MessageChannel code was still on the stack
-            if (!mThat.mListener)
-                return;
-
-            mThat.AssertWorkerThread();
-            if (exitingCall)
-                mThat.ExitedCall();
-
-            if (exitingStack)
-                mThat.ExitedCxxStack();
-        }
-      private:
-        MessageChannel& mThat;
-
-        // disable harmful methods
-        CxxStackFrame();
-        CxxStackFrame(const CxxStackFrame&);
-        CxxStackFrame& operator=(const CxxStackFrame&);
-    };
-
     void DebugAbort(const char* file, int line, const char* cond,
                     const char* why,
                     bool reply=false) const;
 
     // This method is only safe to call on the worker thread, or in a
     // debugger with all threads paused.
     void DumpInterruptStack(const char* const pfx="") const;
 
@@ -660,17 +590,17 @@ class MessageChannel : HasResultCodes
     // Approximation of code frames on the C++ stack. It can only be
     // interpreted as the implication:
     //
     //  !mCxxStackFrames.empty() => MessageChannel code on C++ stack
     //
     // This member is only accessed on the worker thread, and so is not
     // protected by mMonitor.  It is managed exclusively by the helper
     // |class CxxStackFrame|.
-    std::vector<InterruptFrame> mCxxStackFrames;
+    mozilla::Vector<InterruptFrame> mCxxStackFrames;
 
     // Did we process an Interrupt out-call during this stack?  Only meaningful in
     // ExitedCxxStack(), from which this variable is reset.
     bool mSawInterruptOutMsg;
 
     // Map of replies received "out of turn", because of Interrupt
     // in-calls racing with replies to outstanding in-calls.  See
     // https://bugzilla.mozilla.org/show_bug.cgi?id=521929.