bug 865314 dont restrict parallel ssl handshakes with unknown spdy state r=jduell
☠☠ backed out by 83a790e5acd8 ☠ ☠
authorPatrick McManus <mcmanus@ducksong.com>
Wed, 24 Apr 2013 20:27:50 -0400
changeset 140762 96a806212cacbe387dce7f657dc3a53cc98070e3
parent 140761 ba1bd4eccd7cac4c426bd8871cf6b1058cc01f18
child 140763 d3a6b7822cdbfe063f9cc75327adbd93abd64761
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell
bugs865314
milestone23.0a1
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 865314 dont restrict parallel ssl handshakes with unknown spdy state r=jduell
netwerk/protocol/http/ConnectionDiagnostics.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/protocol/http/ConnectionDiagnostics.cpp
+++ b/netwerk/protocol/http/ConnectionDiagnostics.cpp
@@ -63,18 +63,18 @@ nsHttpConnectionMgr::PrintDiagnosticsCB(
   self->mLogData.AppendPrintf("   Active Conns Length = %u\n",
                               ent->mActiveConns.Length());
   self->mLogData.AppendPrintf("   Idle Conns Length = %u\n",
                               ent->mIdleConns.Length());
   self->mLogData.AppendPrintf("   Half Opens Length = %u\n",
                               ent->mHalfOpens.Length());
   self->mLogData.AppendPrintf("   Coalescing Key = %s\n",
                               ent->mCoalescingKey.get());
-  self->mLogData.AppendPrintf("   Spdy using = %d, tested = %d, preferred = %d\n",
-                              ent->mUsingSpdy, ent->mTestedSpdy, ent->mSpdyPreferred);
+  self->mLogData.AppendPrintf("   Spdy using = %d, preferred = %d\n",
+                              ent->mUsingSpdy, ent->mSpdyPreferred);
   self->mLogData.AppendPrintf("   pipelinestate = %d penalty = %d\n",
                               ent->mPipelineState, ent->mPipeliningPenalty);
   for (i = 0; i < nsAHttpTransaction::CLASS_MAX; ++i) {
     self->mLogData.AppendPrintf("   pipeline per class penalty 0x%x %d\n",
                                 i, ent->mPipeliningClassPenalty[i]);
   }
   for (i = 0; i < ent->mActiveConns.Length(); ++i) {
     self->mLogData.AppendPrintf("   :: Active Connection #%u\n", i);
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -516,18 +516,16 @@ nsHttpConnectionMgr::ReportSpdyConnectio
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     
     nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
                                                    conn, nullptr);
 
     if (!ent)
         return;
 
-    ent->mTestedSpdy = true;
-
     if (!usingSpdy)
         return;
 
     ent->mUsingSpdy = true;
     mNumSpdyActiveConns++;
 
     uint32_t ttl = conn->TimeToLive();
     uint64_t timeOfExpire = NowInSeconds() + ttl;
@@ -559,17 +557,38 @@ nsHttpConnectionMgr::ReportSpdyConnectio
         // this connection was negotiating NPN.
 
         // Call don't reuse on the current connection to shut it down as soon
         // as possible without causing any errors.
         // i.e. the current transaction(s) on this connection will be processed
         // normally, but then it will go away and future connections will be
         // coalesced through the preferred entry.
 
-        conn->DontReuse();
+      LOG(("ReportSpdyConnection shutting down connection because "
+	   "of desharding\n"));
+      conn->DontReuse();
+    }
+    else if (ent->mActiveConns.Length() > 1) {
+      // this is a new connection to an established preferred spdy host.
+      // if there is more than 1 live and established spdy connection (e.g.
+      // some could still be handshaking, shutting down, etc..) then close
+      // this one down after any transactions that are on it are complete.
+      // This probably happened due to the parallel connection algorithm
+      // that is used only before the host is known to speak spdy.
+      for (uint32_t index = 0; index < ent->mActiveConns.Length(); ++index) {
+	nsHttpConnection *otherConn = ent->mActiveConns[index];
+	if (otherConn == conn)
+	  continue;
+	if (conn->CanDirectlyActivate()) {
+	  LOG(("ReportSpdyConnection shutting down connection because more "
+	       "than 1 active spdy connection is live to this host\n"));
+	  conn->DontReuse();
+	  break;
+	}
+      }
     }
 
     PostEvent(&nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ);
 }
 
 void
 nsHttpConnectionMgr::ReportSpdyCWNDSetting(nsHttpConnectionInfo *ci,
                                            uint32_t cwndValue)
@@ -946,19 +965,18 @@ nsHttpConnectionMgr::PruneDeadConnection
     // red condition, then we can clean it up and restart from
     // yellow
     if (ent->PipelineState()       != PS_RED &&
         self->mCT.Count()          >  125 &&
         ent->mIdleConns.Length()   == 0 &&
         ent->mActiveConns.Length() == 0 &&
         ent->mHalfOpens.Length()   == 0 &&
         ent->mPendingQ.Length()    == 0 &&
-        ((!ent->mTestedSpdy && !ent->mUsingSpdy) ||
-         !gHttpHandler->IsSpdyEnabled() ||
-         self->mCT.Count() > 300)) {
+        !ent->mUsingSpdy &&
+	self->mCT.Count() > 300) {
         LOG(("    removing empty connection entry\n"));
         return PL_DHASH_REMOVE;
     }
 
     // otherwise use this opportunity to compact our arrays...
     ent->mIdleConns.Compact();
     ent->mActiveConns.Compact();
     ent->mPendingQ.Compact();
@@ -1284,17 +1302,17 @@ nsHttpConnectionMgr::RestrictConnections
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
     // If this host is trying to negotiate a SPDY session right now,
     // don't create any new ssl connections until the result of the
     // negotiation is known.
     
     bool doRestrict = ent->mConnInfo->UsingSSL() &&
         gHttpHandler->IsSpdyEnabled() &&
-        (!ent->mTestedSpdy || ent->mUsingSpdy) &&
+        ent->mUsingSpdy &&
         (ent->mHalfOpens.Length() || ent->mActiveConns.Length());
 
     // If there are no restrictions, we are done
     if (!doRestrict)
         return false;
     
     // If the restriction is based on a tcp handshake in progress
     // let that connect and then see if it was SPDY or not
@@ -3099,17 +3117,16 @@ nsConnectionEntry::nsConnectionEntry(nsH
     , mPipelineState(PS_YELLOW)
     , mYellowGoodEvents(0)
     , mYellowBadEvents(0)
     , mYellowConnection(nullptr)
     , mGreenDepth(kPipelineOpen)
     , mPipeliningPenalty(0)
     , mSpdyCWND(0)
     , mUsingSpdy(false)
-    , mTestedSpdy(false)
     , mSpdyPreferred(false)
     , mPreferIPv4(false)
     , mPreferIPv6(false)
 {
     NS_ADDREF(mConnInfo);
     if (gHttpHandler->GetPipelineAggressive()) {
         mGreenDepth = kPipelineUnlimited;
         mPipelineState = PS_GREEN;
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -353,22 +353,16 @@ private:
         uint32_t            mSpdyCWND;
         mozilla::TimeStamp  mSpdyCWNDTimeStamp;
 
         // To have the UsingSpdy flag means some host with the same connection
         // entry has done NPN=spdy/* at some point. It does not mean every
         // connection is currently using spdy.
         bool mUsingSpdy;
 
-        // mTestedSpdy is set after NPN negotiation has occurred and we know
-        // with confidence whether a host speaks spdy or not (which is reflected
-        // in mUsingSpdy). Before mTestedSpdy is set, handshake parallelism is
-        // minimized so that we can multiplex on a single spdy connection.
-        bool mTestedSpdy;
-
         bool mSpdyPreferred;
 
         // Flags to remember our happy-eyeballs decision.
         // Reset only by Ctrl-F5 reload.
         // True when we've first connected an IPv4 server for this host,
         // initially false.
         bool mPreferIPv4 : 1;
         // True when we've first connected an IPv6 server for this host,