Back out 96a806212cac (bug 865314) for apparently causing fairly frequent failures in test_spdy.js
authorPhil Ringnalda <philringnalda@gmail.com>
Wed, 24 Apr 2013 23:45:40 -0700
changeset 129847 83a790e5acd8a9b1b4b1a13ee39b15a7adc751e1
parent 129846 8a9a40bfa8e3ad6c79333ec63a541dce0f503c44
child 129848 7a0df791c0f8d8ed0ca49bcd5d17a49a9f57ca65
push id1550
push userttaubert@mozilla.com
push dateFri, 26 Apr 2013 12:46:41 +0000
treeherderfx-team@d360244c69ab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs865314
milestone23.0a1
backs out96a806212cacbe387dce7f657dc3a53cc98070e3
Back out 96a806212cac (bug 865314) for apparently causing fairly frequent failures in test_spdy.js
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, preferred = %d\n",
-                              ent->mUsingSpdy, ent->mSpdyPreferred);
+  self->mLogData.AppendPrintf("   Spdy using = %d, tested = %d, preferred = %d\n",
+                              ent->mUsingSpdy, ent->mTestedSpdy, 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,16 +516,18 @@ 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;
@@ -557,38 +559,17 @@ 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.
 
-      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;
-	}
-      }
+        conn->DontReuse();
     }
 
     PostEvent(&nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ);
 }
 
 void
 nsHttpConnectionMgr::ReportSpdyCWNDSetting(nsHttpConnectionInfo *ci,
                                            uint32_t cwndValue)
@@ -965,18 +946,19 @@ 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->mUsingSpdy &&
-	self->mCT.Count() > 300) {
+        ((!ent->mTestedSpdy && !ent->mUsingSpdy) ||
+         !gHttpHandler->IsSpdyEnabled() ||
+         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();
@@ -1302,17 +1284,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->mUsingSpdy &&
+        (!ent->mTestedSpdy || 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
@@ -3117,16 +3099,17 @@ 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,16 +353,22 @@ 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,