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 140791 83a790e5acd8a9b1b4b1a13ee39b15a7adc751e1
parent 140790 8a9a40bfa8e3ad6c79333ec63a541dce0f503c44
child 140792 7a0df791c0f8d8ed0ca49bcd5d17a49a9f57ca65
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)
bugs865314
milestone23.0a1
backs out96a806212cacbe387dce7f657dc3a53cc98070e3
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
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,