Bug 561694: Optimize IPDL traffic for necko OnProgress/OnStatus. r=honzab
authorJason Duell <jduell.mcbugs@gmail.com>
Sat, 09 Apr 2011 20:42:05 -0700
changeset 67800 cd18613f8de4554de20675ba6e5ad3fe7ea97abd
parent 67799 52e0b9902f48ccf739271dcf4d89f0d7a6b8b71b
child 67801 49835a54196a17fa22aec0543274aa69793c0d5f
push id19428
push usereakhgari@mozilla.com
push dateSun, 10 Apr 2011 19:11:44 +0000
treeherdermozilla-central@6eaee284fdb9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs561694
milestone2.2a1pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 561694: Optimize IPDL traffic for necko OnProgress/OnStatus. r=honzab
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.h
netwerk/protocol/http/PHttpChannel.ipdl
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -268,76 +268,125 @@ HttpChannelChild::OnStartRequest(const n
   if (mResponseHead)
     SetCookie(mResponseHead->PeekHeader(nsHttp::Set_Cookie));
 
   rv = ApplyContentConversions();
   if (NS_FAILED(rv))
     Cancel(rv);
 }
 
-class DataAvailableEvent : public ChannelEvent
+class TransportAndDataEvent : public ChannelEvent
 {
  public:
-  DataAvailableEvent(HttpChannelChild* child,
-                     const nsCString& data,
-                     const PRUint32& offset,
-                     const PRUint32& count)
+  TransportAndDataEvent(HttpChannelChild* child,
+                        const nsresult& status,
+                        const PRUint64& progress,
+                        const PRUint64& progressMax,
+                        const nsCString& data,
+                        const PRUint32& offset,
+                        const PRUint32& count)
   : mChild(child)
+  , mStatus(status)
+  , mProgress(progress)
+  , mProgressMax(progressMax)
   , mData(data)
   , mOffset(offset)
   , mCount(count) {}
 
-  void Run() { mChild->OnDataAvailable(mData, mOffset, mCount); }
+  void Run() { mChild->OnTransportAndData(mStatus, mProgress, mProgressMax,
+                                          mData, mOffset, mCount); }
  private:
   HttpChannelChild* mChild;
+  nsresult mStatus;
+  PRUint64 mProgress;
+  PRUint64 mProgressMax;
   nsCString mData;
   PRUint32 mOffset;
   PRUint32 mCount;
 };
 
-bool 
-HttpChannelChild::RecvOnDataAvailable(const nsCString& data,
-                                      const PRUint32& offset,
-                                      const PRUint32& count)
+bool
+HttpChannelChild::RecvOnTransportAndData(const nsresult& status,
+                                         const PRUint64& progress,
+                                         const PRUint64& progressMax,
+                                         const nsCString& data,
+                                         const PRUint32& offset,
+                                         const PRUint32& count)
 {
   if (ShouldEnqueue()) {
-    EnqueueEvent(new DataAvailableEvent(this, data, offset, count));
+    EnqueueEvent(new TransportAndDataEvent(this, status, progress, progressMax,
+                                           data, offset, count));
   } else {
-    OnDataAvailable(data, offset, count);
+    OnTransportAndData(status, progress, progressMax, data, offset, count);
   }
   return true;
 }
 
-void 
-HttpChannelChild::OnDataAvailable(const nsCString& data,
-                                  const PRUint32& offset,
-                                  const PRUint32& count)
+void
+HttpChannelChild::OnTransportAndData(const nsresult& status,
+                                     const PRUint64 progress,
+                                     const PRUint64& progressMax,
+                                     const nsCString& data,
+                                     const PRUint32& offset,
+                                     const PRUint32& count)
 {
-  LOG(("HttpChannelChild::OnDataAvailable [this=%x]\n", this));
+  LOG(("HttpChannelChild::OnTransportAndData [this=%x]\n", this));
 
   if (mCanceled)
     return;
 
+  // cache the progress sink so we don't have to query for it each time.
+  if (!mProgressSink)
+    GetCallback(mProgressSink);
+
+  // Hold queue lock throughout all three calls, else we might process a later
+  // necko msg in between them.
+  AutoEventEnqueuer ensureSerialDispatch(this);
+
+  // block status/progress after Cancel or OnStopRequest has been called,
+  // or if channel has LOAD_BACKGROUND set.
+  // - JDUELL: may not need mStatus/mIsPending checks, given this is always called
+  //   during OnDataAvailable, and we've already checked mCanceled.  Code
+  //   dupe'd from nsHttpChannel
+  if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending &&
+      !(mLoadFlags & LOAD_BACKGROUND))
+  {
+    // OnStatus
+    //
+    NS_ASSERTION(status == nsISocketTransport::STATUS_RECEIVING_FROM ||
+                 status == nsITransport::STATUS_READING,
+                 "unexpected status code");
+
+    nsCAutoString host;
+    mURI->GetHost(host);
+    mProgressSink->OnStatus(this, nsnull, status,
+                            NS_ConvertUTF8toUTF16(host).get());
+    // OnProgress
+    //
+    if (progress > 0) {
+      NS_ASSERTION(progress <= progressMax, "unexpected progress values");
+      mProgressSink->OnProgress(this, nsnull, progress, progressMax);
+    }
+  }
+
+  // OnDataAvailable
+  //
   // NOTE: the OnDataAvailable contract requires the client to read all the data
   // in the inputstream.  This code relies on that ('data' will go away after
   // this function).  Apparently the previous, non-e10s behavior was to actually
   // support only reading part of the data, allowing later calls to read the
   // rest.
   nsCOMPtr<nsIInputStream> stringStream;
-  nsresult rv = NS_NewByteInputStream(getter_AddRefs(stringStream),
-                                      data.get(),
-                                      count,
-                                      NS_ASSIGNMENT_DEPEND);
+  nsresult rv = NS_NewByteInputStream(getter_AddRefs(stringStream), data.get(),
+                                      count, NS_ASSIGNMENT_DEPEND);
   if (NS_FAILED(rv)) {
     Cancel(rv);
     return;
   }
 
-  AutoEventEnqueuer ensureSerialDispatch(this);
-
   rv = mListener->OnDataAvailable(this, mListenerContext,
                                   stringStream, offset, count);
   stringStream->Close();
   if (NS_FAILED(rv)) {
     Cancel(rv);
   }
 }
 
@@ -442,76 +491,76 @@ HttpChannelChild::OnProgress(const PRUin
     return;
 
   // cache the progress sink so we don't have to query for it each time.
   if (!mProgressSink)
     GetCallback(mProgressSink);
 
   AutoEventEnqueuer ensureSerialDispatch(this);
 
-  // block socket status event after Cancel or OnStopRequest has been called.
+  // block socket status event after Cancel or OnStopRequest has been called,
+  // or if channel has LOAD_BACKGROUND set
   if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending && 
       !(mLoadFlags & LOAD_BACKGROUND)) 
   {
     if (progress > 0) {
       NS_ASSERTION(progress <= progressMax, "unexpected progress values");
       mProgressSink->OnProgress(this, nsnull, progress, progressMax);
     }
   }
 }
 
 class StatusEvent : public ChannelEvent
 {
  public:
   StatusEvent(HttpChannelChild* child,
-              const nsresult& status,
-              const nsString& statusArg)
+              const nsresult& status)
   : mChild(child)
-  , mStatus(status)
-  , mStatusArg(statusArg) {}
+  , mStatus(status) {}
 
-  void Run() { mChild->OnStatus(mStatus, mStatusArg); }
+  void Run() { mChild->OnStatus(mStatus); }
  private:
   HttpChannelChild* mChild;
   nsresult mStatus;
-  nsString mStatusArg;
 };
 
 bool
-HttpChannelChild::RecvOnStatus(const nsresult& status,
-                               const nsString& statusArg)
+HttpChannelChild::RecvOnStatus(const nsresult& status)
 {
   if (ShouldEnqueue()) {
-    EnqueueEvent(new StatusEvent(this, status, statusArg));
+    EnqueueEvent(new StatusEvent(this, status));
   } else {
-    OnStatus(status, statusArg);
+    OnStatus(status);
   }
   return true;
 }
 
 void
-HttpChannelChild::OnStatus(const nsresult& status,
-                           const nsString& statusArg)
+HttpChannelChild::OnStatus(const nsresult& status)
 {
   LOG(("HttpChannelChild::OnStatus [this=%p status=%x]\n", this, status));
 
   if (mCanceled)
     return;
 
   // cache the progress sink so we don't have to query for it each time.
   if (!mProgressSink)
     GetCallback(mProgressSink);
 
   AutoEventEnqueuer ensureSerialDispatch(this);
-  
-  // block socket status event after Cancel or OnStopRequest has been called.
+
+  // block socket status event after Cancel or OnStopRequest has been called,
+  // or if channel has LOAD_BACKGROUND set
   if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending && 
       !(mLoadFlags & LOAD_BACKGROUND)) 
   {
-    mProgressSink->OnStatus(this, nsnull, status, statusArg.get());
+    nsCAutoString host;
+    mURI->GetHost(host);
+    mProgressSink->OnStatus(this, nsnull, status,
+                            NS_ConvertUTF8toUTF16(host).get());
   }
 }
 
 class CancelEvent : public ChannelEvent
 {
  public:
   CancelEvent(HttpChannelChild* child, const nsresult& status)
   : mChild(child)
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -126,22 +126,25 @@ protected:
   bool RecvOnStartRequest(const nsHttpResponseHead& responseHead,
                           const PRBool& useResponseHead,
                           const RequestHeaderTuples& requestHeaders,
                           const PRBool& isFromCache,
                           const PRBool& cacheEntryAvailable,
                           const PRUint32& cacheExpirationTime,
                           const nsCString& cachedCharset,
                           const nsCString& securityInfoSerialization);
-  bool RecvOnDataAvailable(const nsCString& data, 
-                           const PRUint32& offset,
-                           const PRUint32& count);
+  bool RecvOnTransportAndData(const nsresult& status,
+                              const PRUint64& progress,
+                              const PRUint64& progressMax,
+                              const nsCString& data,
+                              const PRUint32& offset,
+                              const PRUint32& count);
   bool RecvOnStopRequest(const nsresult& statusCode);
   bool RecvOnProgress(const PRUint64& progress, const PRUint64& progressMax);
-  bool RecvOnStatus(const nsresult& status, const nsString& statusArg);
+  bool RecvOnStatus(const nsresult& status);
   bool RecvCancelEarly(const nsresult& status);
   bool RecvRedirect1Begin(const PRUint32& newChannel,
                           const URI& newURI,
                           const PRUint32& redirectFlags,
                           const nsHttpResponseHead& responseHead);
   bool RecvRedirect3Complete();
   bool RecvAssociateApplicationCache(const nsCString& groupID,
                                      const nsCString& clientID);
@@ -164,40 +167,43 @@ private:
   bool mSendResumeAt;
   // Current suspension depth for this channel object
   PRUint32 mSuspendCount;
 
   bool mIPCOpen;
   bool mKeptAlive;
 
   void OnStartRequest(const nsHttpResponseHead& responseHead,
-                          const PRBool& useResponseHead,
-                          const RequestHeaderTuples& requestHeaders,
-                          const PRBool& isFromCache,
-                          const PRBool& cacheEntryAvailable,
-                          const PRUint32& cacheExpirationTime,
-                          const nsCString& cachedCharset,
-                          const nsCString& securityInfoSerialization);
-  void OnDataAvailable(const nsCString& data, 
-                       const PRUint32& offset,
-                       const PRUint32& count);
+                      const PRBool& useResponseHead,
+                      const RequestHeaderTuples& requestHeaders,
+                      const PRBool& isFromCache,
+                      const PRBool& cacheEntryAvailable,
+                      const PRUint32& cacheExpirationTime,
+                      const nsCString& cachedCharset,
+                      const nsCString& securityInfoSerialization);
+  void OnTransportAndData(const nsresult& status,
+                          const PRUint64 progress,
+                          const PRUint64& progressMax,
+                          const nsCString& data,
+                          const PRUint32& offset,
+                          const PRUint32& count);
   void OnStopRequest(const nsresult& statusCode);
   void OnProgress(const PRUint64& progress, const PRUint64& progressMax);
-  void OnStatus(const nsresult& status, const nsString& statusArg);
+  void OnStatus(const nsresult& status);
   void OnCancel(const nsresult& status);
   void Redirect1Begin(const PRUint32& newChannelId,
                       const URI& newUri,
                       const PRUint32& redirectFlags,
                       const nsHttpResponseHead& responseHead);
   void Redirect3Complete();
   void DeleteSelf();
 
   friend class StartRequestEvent;
   friend class StopRequestEvent;
-  friend class DataAvailableEvent;
+  friend class TransportAndDataEvent;
   friend class ProgressEvent;
   friend class StatusEvent;
   friend class CancelEvent;
   friend class Redirect1Event;
   friend class Redirect3Event;
   friend class DeleteSelfEvent;
 };
 
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -57,17 +57,20 @@
 #include "nsIApplicationCacheService.h"
 #include "nsIOfflineCacheUpdate.h"
 #include "nsIRedirectChannelRegistrar.h"
 
 namespace mozilla {
 namespace net {
 
 HttpChannelParent::HttpChannelParent(PBrowserParent* iframeEmbedding)
-: mIPCClosed(false)
+  : mIPCClosed(false)
+  , mStoredStatus(0)
+  , mStoredProgress(0)
+  , mStoredProgressMax(0)
 {
   // Ensure gHttpHandler is initialized: we need the atom table up and running.
   nsIHttpProtocolHandler* handler;
   CallGetService(NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "http", &handler);
   NS_ASSERTION(handler, "no http handler");
 
   mTabParent = do_QueryInterface(static_cast<nsITabParent*>(
       static_cast<TabParent*>(iframeEmbedding)));
@@ -460,50 +463,77 @@ HttpChannelParent::OnStopRequest(nsIRequ
 NS_IMETHODIMP
 HttpChannelParent::OnDataAvailable(nsIRequest *aRequest, 
                                    nsISupports *aContext, 
                                    nsIInputStream *aInputStream, 
                                    PRUint32 aOffset, 
                                    PRUint32 aCount)
 {
   LOG(("HttpChannelParent::OnDataAvailable [this=%x]\n", this));
- 
+
   nsCString data;
   nsresult rv = NS_ReadInputStreamToString(aInputStream, data, aCount);
   if (NS_FAILED(rv))
     return rv;
 
-  if (mIPCClosed || !SendOnDataAvailable(data, aOffset, aCount))
-    return NS_ERROR_UNEXPECTED; 
-
+  // OnDataAvailable is always preceded by OnStatus/OnProgress calls that set
+  // mStoredStatus/mStoredProgress(Max) to appropriate values, unless
+  // LOAD_BACKGROUND set.  In that case, they'll have garbage values, but
+  // child doesn't use them.
+  if (mIPCClosed || !SendOnTransportAndData(mStoredStatus, mStoredProgress,
+                                            mStoredProgressMax, data, aOffset,
+                                            aCount)) {
+    return NS_ERROR_UNEXPECTED;
+  }
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsIProgressEventSink
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 HttpChannelParent::OnProgress(nsIRequest *aRequest, 
                               nsISupports *aContext, 
                               PRUint64 aProgress, 
                               PRUint64 aProgressMax)
 {
-  if (mIPCClosed || !SendOnProgress(aProgress, aProgressMax))
-    return NS_ERROR_UNEXPECTED;
+  // OnStatus has always just set mStoredStatus. If it indicates this precedes
+  // OnDataAvailable, store and ODA will send to child.
+  if (mStoredStatus == nsISocketTransport::STATUS_RECEIVING_FROM ||
+      mStoredStatus == nsITransport::STATUS_READING)
+  {
+    mStoredProgress = aProgress;
+    mStoredProgressMax = aProgressMax;
+  } else {
+    // Send to child now.  The only case I've observed that this handles (i.e.
+    // non-ODA status with progress > 0) is data upload progress notification
+    // (status == nsISocketTransport::STATUS_SENDING_TO)
+    if (mIPCClosed || !SendOnProgress(aProgress, aProgressMax))
+      return NS_ERROR_UNEXPECTED;
+  }
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 HttpChannelParent::OnStatus(nsIRequest *aRequest, 
                             nsISupports *aContext, 
                             nsresult aStatus, 
                             const PRUnichar *aStatusArg)
 {
-  if (mIPCClosed || !SendOnStatus(aStatus, nsString(aStatusArg)))
+  // If this precedes OnDataAvailable, store and ODA will send to child.
+  if (aStatus == nsISocketTransport::STATUS_RECEIVING_FROM ||
+      aStatus == nsITransport::STATUS_READING)
+  {
+    mStoredStatus = aStatus;
+    return NS_OK;
+  }
+  // Otherwise, send to child now
+  if (mIPCClosed || !SendOnStatus(aStatus))
     return NS_ERROR_UNEXPECTED;
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsIParentChannel
 //-----------------------------------------------------------------------------
 
--- a/netwerk/protocol/http/HttpChannelParent.h
+++ b/netwerk/protocol/http/HttpChannelParent.h
@@ -119,14 +119,20 @@ protected:
 
 private:
   nsCOMPtr<nsIChannel> mChannel;
   nsCOMPtr<nsICacheEntryDescriptor> mCacheDescriptor;
   bool mIPCClosed;                // PHttpChannel actor has been Closed()
 
   nsCOMPtr<nsIChannel> mRedirectChannel;
   nsCOMPtr<nsIAsyncVerifyRedirectCallback> mRedirectCallback;
+
+  // state for combining OnStatus/OnProgress with OnDataAvailable
+  // into one IPDL call to child.
+  nsresult mStoredStatus;
+  PRUint64 mStoredProgress;
+  PRUint64 mStoredProgressMax;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_HttpChannelParent_h
--- a/netwerk/protocol/http/PHttpChannel.ipdl
+++ b/netwerk/protocol/http/PHttpChannel.ipdl
@@ -76,16 +76,18 @@ parent:
             PRBool              allowPipelining,
             PRBool              forceAllowThirdPartyCookie,
             bool                resumeAt,
             PRUint64            startPos,
             nsCString           entityID,
             bool                chooseApplicationCache,
             nsCString           appCacheClientID);
 
+  // Used to connect redirected-to channel on the parent with redirected-to
+  // channel on the child.
   ConnectChannel(PRUint32 channelId);
 
   SetPriority(PRUint16 priority);
 
   SetCacheTokenCachedCharset(nsCString charset);
 
   UpdateAssociatedContentSecurity(PRInt32 high,
                                   PRInt32 low,
@@ -126,25 +128,30 @@ child:
                  PRBool              useResponseHead,
                  RequestHeaderTuples requestHeaders,
                  PRBool              isFromCache,
                  PRBool              cacheEntryAvailable,
                  PRUint32            cacheExpirationTime,
                  nsCString           cachedCharset,
                  nsCString           securityInfoSerialization);
 
-  OnDataAvailable(nsCString data, 
-                  PRUint32  offset, 
-                  PRUint32  count);
+  // Combines a single OnDataAvailable and its associated OnProgress &
+  // OnStatus calls into one IPDL message
+  OnTransportAndData(nsresult  status,
+                     PRUint64  progress,
+                     PRUint64  progressMax,
+                     nsCString data,
+                     PRUint32  offset,
+                     PRUint32  count);
 
   OnStopRequest(nsresult statusCode);
 
   OnProgress(PRUint64 progress, PRUint64 progressMax);
 
-  OnStatus(nsresult status, nsString statusArg);
+  OnStatus(nsresult status);
 
   // Used to cancel child channel if we hit errors during creating and
   // AsyncOpen of nsHttpChannel on the parent.
   CancelEarly(nsresult status);
 
   // Called to initiate content channel redirect, starts talking to sinks
   // on the content process and reports result via Redirect2Verify above
   Redirect1Begin(PRUint32           newChannelId,