bug 865314 - backout for unused variable compiler bustage r=bustage CLOSED TREE
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 24 Oct 2014 11:52:18 -0400
changeset 212259 5fcda0c427935b3ead2b8e8cbfab2374efc7562d
parent 212258 b7050902ed0311a331b3886f1ec3e6aa1a97f14f
child 212260 c70f62375f7d496edee7ebd2535fce746500e96c
child 212271 853612ddc6861ea705848979a03ed23f35d66f35
push id27702
push userkwierso@gmail.com
push dateFri, 24 Oct 2014 22:05:50 +0000
treeherdermozilla-central@c70f62375f7d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbustage
bugs865314
milestone36.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 - backout for unused variable compiler bustage r=bustage CLOSED TREE
netwerk/base/public/nsISpeculativeConnect.idl
netwerk/base/src/Predictor.cpp
netwerk/protocol/http/AlternateServices.cpp
netwerk/protocol/http/ConnectionDiagnostics.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/base/public/nsISpeculativeConnect.idl
+++ b/netwerk/base/public/nsISpeculativeConnect.idl
@@ -30,26 +30,33 @@ interface nsISpeculativeConnect : nsISup
 
 };
 
 /**
  * This is used to override the default values for various values (documented
  * inline) to determine whether or not to actually make a speculative
  * connection.
  */
-[builtinclass, uuid(1040ebe3-6ed1-45a6-8587-995e082518d7)]
+[builtinclass, uuid(f6a0d1e5-369f-4abc-81ae-d370d36e4006)]
 interface nsISpeculativeConnectionOverrider : nsISupports
 {
     /**
      * Used to determine the maximum number of unused speculative connections
      * we will have open for a host at any one time
      */
     [infallible] readonly attribute unsigned long parallelSpeculativeConnectLimit;
 
     /**
+     * Used to loosen the restrictions nsHttpConnectionMgr::RestrictConnections
+     * to allow more speculative connections when we're unsure if a host will
+     * connect via SPDY or not.
+     */
+    [infallible] readonly attribute boolean ignorePossibleSpdyConnections;
+
+    /**
      * Used to determine if we will ignore the existence of any currently idle
      * connections when we decide whether or not to make a speculative
      * connection.
      */
     [infallible] readonly attribute boolean ignoreIdle;
 
     /*
      * Used by the Predictor to gather telemetry data on speculative connection
--- a/netwerk/base/src/Predictor.cpp
+++ b/netwerk/base/src/Predictor.cpp
@@ -369,16 +369,23 @@ Predictor::Observe(nsISupports *subject,
 NS_IMETHODIMP
 Predictor::GetIgnoreIdle(bool *ignoreIdle)
 {
   *ignoreIdle = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
+Predictor::GetIgnorePossibleSpdyConnections(bool *ignorePossibleSpdyConnections)
+{
+  *ignorePossibleSpdyConnections = true;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 Predictor::GetParallelSpeculativeConnectLimit(
     uint32_t *parallelSpeculativeConnectLimit)
 {
   *parallelSpeculativeConnectLimit = 6;
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/netwerk/protocol/http/AlternateServices.cpp
+++ b/netwerk/protocol/http/AlternateServices.cpp
@@ -410,16 +410,23 @@ AltSvcOverride::GetInterface(const nsIID
 NS_IMETHODIMP
 AltSvcOverride::GetIgnoreIdle(bool *ignoreIdle)
 {
   *ignoreIdle = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
+AltSvcOverride::GetIgnorePossibleSpdyConnections(bool *ignorePossibleSpdyConnections)
+{
+  *ignorePossibleSpdyConnections = true;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 AltSvcOverride::GetParallelSpeculativeConnectLimit(
   uint32_t *parallelSpeculativeConnectLimit)
 {
   *parallelSpeculativeConnectLimit = 32;
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/netwerk/protocol/http/ConnectionDiagnostics.cpp
+++ b/netwerk/protocol/http/ConnectionDiagnostics.cpp
@@ -68,18 +68,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/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -154,17 +154,16 @@ void
 nsHttpConnection::StartSpdy(uint8_t spdyVersion)
 {
     LOG(("nsHttpConnection::StartSpdy [this=%p]\n", this));
 
     MOZ_ASSERT(!mSpdySession);
 
     mUsingSpdyVersion = spdyVersion;
     mEverUsedSpdy = true;
-    mSpdySession = ASpdySession::NewSpdySession(spdyVersion, mSocketTransport);
 
     if (!mReportedSpdy) {
         mReportedSpdy = true;
         gHttpHandler->ConnMgr()->ReportSpdyConnection(this, true);
     }
 
     // Setting the connection as reused allows some transactions that fail
     // with NS_ERROR_NET_RESET to be restarted and SPDY uses that code
@@ -205,16 +204,17 @@ nsHttpConnection::StartSpdy(uint8_t spdy
               "Proxy and Need Connect", this));
         MOZ_ASSERT(mProxyConnectStream);
 
         mProxyConnectStream = nullptr;
         mCompletedProxyConnect = true;
         mProxyConnectInProgress = false;
     }
 
+    mSpdySession = ASpdySession::NewSpdySession(spdyVersion, mSocketTransport);
     bool spdyProxy = mConnInfo->UsingHttpsProxy() && !mTLSFilter;
     if (spdyProxy) {
         nsRefPtr<nsHttpConnectionInfo> wildCardProxyCi;
         mConnInfo->CreateWildCard(getter_AddRefs(wildCardProxyCi));
         gHttpHandler->ConnMgr()->MoveToWildCardConnEntry(mConnInfo,
                                                          wildCardProxyCi, this);
         mConnInfo = wildCardProxyCi;
     }
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -375,16 +375,17 @@ public:
     NS_IMETHOD_(MozExternalRefCountType) Release(void);
 
 public: // intentional!
     nsRefPtr<NullHttpTransaction> mTrans;
 
     bool mOverridesOK;
     uint32_t mParallelSpeculativeConnectLimit;
     bool mIgnoreIdle;
+    bool mIgnorePossibleSpdyConnections;
     bool mIsFromPredictor;
     bool mAllow1918;
 
     // As above, added manually so we can use nsRefPtr without inheriting from
     // nsISupports
 protected:
     ThreadSafeAutoRefCnt mRefCnt;
     NS_DECL_OWNINGTHREAD
@@ -431,16 +432,18 @@ nsHttpConnectionMgr::SpeculativeConnect(
     args->mTrans =
         nullTransaction ? nullTransaction : new NullHttpTransaction(ci, wrappedCallbacks, caps);
 
     if (overrider) {
         args->mOverridesOK = true;
         overrider->GetParallelSpeculativeConnectLimit(
             &args->mParallelSpeculativeConnectLimit);
         overrider->GetIgnoreIdle(&args->mIgnoreIdle);
+        overrider->GetIgnorePossibleSpdyConnections(
+            &args->mIgnorePossibleSpdyConnections);
         overrider->GetIsFromPredictor(&args->mIsFromPredictor);
         overrider->GetAllow1918(&args->mAllow1918);
     }
 
     nsresult rv =
         PostEvent(&nsHttpConnectionMgr::OnMsgSpeculativeConnect, 0, args);
     if (NS_SUCCEEDED(rv))
         unused << args.forget();
@@ -647,16 +650,18 @@ nsHttpConnectionMgr::ReportSpdyConnectio
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
 
     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;
@@ -667,89 +672,44 @@ nsHttpConnectionMgr::ReportSpdyConnectio
     // GetSpdyPreferredEnt() because we want to avoid the cert compatibility
     // check at this point because the cert is never part of the hash
     // lookup. Filtering on that has to be done at the time of use
     // rather than the time of registration (i.e. now).
     nsConnectionEntry *joinedConnection;
     nsConnectionEntry *preferred =
         mSpdyPreferredHash.Get(ent->mCoalescingKey);
 
-    LOG(("ReportSpdyConnection conn=%p %s %s ent=%p preferred=%p\n",conn,
-         ent->mConnInfo->Host(), ent->mCoalescingKey.get(), ent, preferred));
+    LOG(("ReportSpdyConnection %s %s ent=%p preferred=%p\n",
+         ent->mConnInfo->Host(), ent->mCoalescingKey.get(),
+         ent, preferred));
 
     if (!preferred) {
-        preferred = ent;
         if (!ent->mCoalescingKey.IsEmpty()) {
             mSpdyPreferredHash.Put(ent->mCoalescingKey, ent);
             ent->mSpdyPreferred = true;
         }
     } else if ((preferred != ent) &&
                (joinedConnection = GetSpdyPreferredEnt(ent)) &&
                (joinedConnection != ent)) {
         //
         // A connection entry (e.g. made with a different hostname) with
         // the same IP address is preferred for future transactions over this
         // connection entry. Gracefully close down the connection to help
         // new transactions migrate over.
 
         LOG(("ReportSpdyConnection graceful close of conn=%p ent=%p to "
-                 "migrate to preferred (desharding)\n", conn, ent));
+                 "migrate to preferred\n", conn, ent));
+
         conn->DontReuse();
     } else if (preferred != ent) {
         LOG (("ReportSpdyConnection preferred host may be in false start or "
               "may have insufficient cert. Leave mapping in place but do not "
               "abandon this connection yet."));
     }
 
-    if (preferred == ent) {
-        // this is a new spdy connection to the preferred entry
-        for (int32_t index = ent->mHalfOpens.Length() - 1;
-             (index >= 0) &&  conn->CanDirectlyActivate(); --index) {
-
-            // Abandon all other half-open sockets belonging to the given transaction.
-            // While activating their transactions on the new conn
-
-            nsHalfOpenSocket *half = ent->mHalfOpens[index];
-            nsAHttpTransaction *abstractTrans = half->Transaction();
-            nsRefPtr<nsHttpTransaction> concreteTrans = abstractTrans->QueryHttpTransaction();
-
-            if (!concreteTrans) {
-                continue;
-            }
-
-            LOG(("ReportSpdyConnection conn %p taking transaction %p from a "
-                 "half open that it will cancel\n", conn, concreteTrans.get()));
-
-            // concreteTrans is holding a ref to the transaction in half - so it
-            // is ok to destroy half
-            ent->RemoveHalfOpen(half);
-            half->Abandon();
-
-            nsresult rv = DispatchTransaction(ent, concreteTrans, conn);
-            MOZ_ASSERT(NS_SUCCEEDED(rv)); // this cannot fail
-        }
-
-        if (conn->CanDirectlyActivate() && 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) {
-                    LOG(("ReportSpdyConnection shutting down connection (%p) because new "
-                         "spdy connection (%p) takes precedence\n", otherConn, conn));
-                    otherConn->DontReuse();
-                }
-            }
-        }
-    }
-
     ProcessPendingQ(ent->mConnInfo);
     PostEvent(&nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ);
 }
 
 void
 nsHttpConnectionMgr::ReportSpdyCWNDSetting(nsHttpConnectionInfo *ci,
                                            uint32_t cwndValue)
 {
@@ -1062,17 +1022,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();
@@ -1442,38 +1404,39 @@ nsHttpConnectionMgr::ClosePersistentConn
                                                   void *closure)
 {
     nsHttpConnectionMgr *self = static_cast<nsHttpConnectionMgr *>(closure);
     self->ClosePersistentConnections(ent);
     return PL_DHASH_NEXT;
 }
 
 bool
-nsHttpConnectionMgr::RestrictConnections(nsConnectionEntry *ent)
+nsHttpConnectionMgr::RestrictConnections(nsConnectionEntry *ent,
+                                         bool ignorePossibleSpdyConnections)
 {
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
 
     // 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->FirstHopSSL() &&
         gHttpHandler->IsSpdyEnabled() &&
-        ent->mUsingSpdy &&
+        ((!ent->mTestedSpdy && !ignorePossibleSpdyConnections) ||
+         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
-    if (ent->UnconnectedHalfOpens()) {
+    if (ent->UnconnectedHalfOpens() && !ignorePossibleSpdyConnections)
         return true;
-    }
 
     // There is a concern that a host is using a mix of HTTP/1 and SPDY.
     // In that case we don't want to restrict connections just because
     // there is a single active HTTP/1 session in use.
     if (ent->mUsingSpdy && ent->mActiveConns.Length()) {
         bool confirmedRestrict = false;
         for (uint32_t index = 0; index < ent->mActiveConns.Length(); ++index) {
             nsHttpConnection *conn = ent->mActiveConns[index];
@@ -2977,32 +2940,34 @@ nsHttpConnectionMgr::OnMsgSpeculativeCon
     // the ip pooling rules. If so, connect to the preferred host instead of
     // the one directly passed in here.
     nsConnectionEntry *preferredEntry = GetSpdyPreferredEnt(ent);
     if (preferredEntry)
         ent = preferredEntry;
 
     uint32_t parallelSpeculativeConnectLimit =
         gHttpHandler->ParallelSpeculativeConnectLimit();
+    bool ignorePossibleSpdyConnections = false;
     bool ignoreIdle = false;
     bool isFromPredictor = false;
     bool allow1918 = false;
 
     if (args->mOverridesOK) {
         parallelSpeculativeConnectLimit = args->mParallelSpeculativeConnectLimit;
+        ignorePossibleSpdyConnections = args->mIgnorePossibleSpdyConnections;
         ignoreIdle = args->mIgnoreIdle;
         isFromPredictor = args->mIsFromPredictor;
         allow1918 = args->mAllow1918;
     }
 
     bool keepAlive = args->mTrans->Caps() & NS_HTTP_ALLOW_KEEPALIVE;
     if (mNumHalfOpenConns < parallelSpeculativeConnectLimit &&
         ((ignoreIdle && (ent->mIdleConns.Length() < parallelSpeculativeConnectLimit)) ||
          !ent->mIdleConns.Length()) &&
-        !(keepAlive && RestrictConnections(ent)) &&
+        !(keepAlive && RestrictConnections(ent, ignorePossibleSpdyConnections)) &&
         !AtActiveConnectionLimit(ent, args->mTrans->Caps())) {
         CreateTransport(ent, args->mTrans, args->mTrans->Caps(), true, isFromPredictor, allow1918);
     }
     else {
         LOG(("  Transport not created due to existing connection count\n"));
     }
 }
 
@@ -3356,19 +3321,18 @@ nsHalfOpenSocket::OnOutputStreamReady(ns
 
         if (NS_SUCCEEDED(mSocketTransport->GetPeerAddr(&peeraddr)))
             mEnt->RecordIPFamilyPreference(peeraddr.raw.family);
 
         // The nsHttpConnection object now owns these streams and sockets
         mStreamOut = nullptr;
         mStreamIn = nullptr;
         mSocketTransport = nullptr;
-    } else {
-        MOZ_ASSERT(!mTransaction->IsNullTransaction(),
-                   "null transactions dont have backup timers");
+    }
+    else {
         TimeDuration rtt = TimeStamp::Now() - mBackupSynStarted;
         rv = conn->Init(mEnt->mConnInfo,
                         gHttpHandler->ConnMgr()->mMaxRequestDelay,
                         mBackupTransport, mBackupStreamIn, mBackupStreamOut,
                         mBackupConnectedOK, callbacks,
                         PR_MillisecondsToInterval(
                           static_cast<uint32_t>(rtt.ToMilliseconds())));
 
@@ -3588,16 +3552,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)
 {
     MOZ_COUNT_CTOR(nsConnectionEntry);
     if (gHttpHandler->GetPipelineAggressive()) {
         mGreenDepth = kPipelineUnlimited;
         mPipelineState = PS_GREEN;
@@ -3985,16 +3950,17 @@ nsHttpConnectionMgr::MoveToWildCardConnE
     }
 
     nsConnectionEntry *wcEnt = GetOrCreateConnectionEntry(wildCardCI, true);
     if (wcEnt == ent) {
         // nothing to do!
         return;
     }
     wcEnt->mUsingSpdy = true;
+    wcEnt->mTestedSpdy = true;
 
     LOG(("nsHttpConnectionMgr::MakeConnEntryWildCard ent %p "
          "idle=%d active=%d half=%d pending=%d\n", ent,
          ent->mIdleConns.Length(), ent->mActiveConns.Length(),
          ent->mHalfOpens.Length(), ent->mPendingQ.Length()));
 
     LOG(("nsHttpConnectionMgr::MakeConnEntryWildCard wc-ent %p "
          "idle=%d active=%d half=%d pending=%d\n", wcEnt,
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -371,16 +371,22 @@ private:
         uint32_t            mSpdyCWND;
         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,
@@ -554,17 +560,17 @@ private:
     nsresult DispatchAbstractTransaction(nsConnectionEntry *,
                                          nsAHttpTransaction *,
                                          uint32_t,
                                          nsHttpConnection *,
                                          int32_t);
     nsresult BuildPipeline(nsConnectionEntry *,
                            nsAHttpTransaction *,
                            nsHttpPipeline **);
-    bool     RestrictConnections(nsConnectionEntry *);
+    bool     RestrictConnections(nsConnectionEntry *, bool = false);
     nsresult ProcessNewTransaction(nsHttpTransaction *);
     nsresult EnsureSocketThreadTarget();
     void     ClosePersistentConnections(nsConnectionEntry *ent);
     void     ReportProxyTelemetry(nsConnectionEntry *ent);
     nsresult CreateTransport(nsConnectionEntry *, nsAHttpTransaction *,
                              uint32_t, bool, bool, bool);
     void     AddActiveConn(nsHttpConnection *, nsConnectionEntry *);
     void     DecrementActiveConnCount(nsHttpConnection *);