Bug 1369496 - HTTP transaction read throttling algorithm improvements. r=mcmanus
authorHonza Bambas <honzab.moz@firemni.cz>
Thu, 08 Jun 2017 09:45:00 -0400
changeset 413602 2b11565d26fc3e84db455ecc46cab3fdf4cde59b
parent 413601 f7d5030aee458b2d47ccdb9d88c6088f16a5c682
child 413603 ed9bcfd60646b4e0def9b5ca80e67fa24d55af58
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1369496
milestone55.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 1369496 - HTTP transaction read throttling algorithm improvements. r=mcmanus
modules/libpref/init/all.js
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/protocol/http/nsHttpTransaction.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2096,22 +2096,22 @@ pref("network.auth.subresource-img-cross
 //
 // This preference has no effect when the browser is set to "Never Remember History",
 // in that case default credentials will always be used.
 pref("network.auth.private-browsing-sso", false);
 
 // Control how throttling of http responses works - number of ms that each
 // suspend and resume period lasts (prefs named appropriately)
 pref("network.http.throttle.enable", true);
-pref("network.http.throttle.suspend-for", 3000);
-pref("network.http.throttle.resume-for", 200);
+pref("network.http.throttle.suspend-for", 900);
+pref("network.http.throttle.resume-for", 100);
 // Delay we resume throttled background responses after the last unthrottled
 // response has finished.  Prevents resuming too soon during an active page load
 // at which sub-resource reqeusts quickly come and go.
-pref("network.http.throttle.resume-background-in", 400);
+pref("network.http.throttle.resume-background-in", 1000);
 
 pref("permissions.default.image",           1); // 1-Accept, 2-Deny, 3-dontAcceptForeign
 
 pref("network.proxy.type",                  5);
 pref("network.proxy.ftp",                   "");
 pref("network.proxy.ftp_port",              0);
 pref("network.proxy.http",                  "");
 pref("network.proxy.http_port",             0);
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -3004,18 +3004,18 @@ nsHttpConnectionMgr::RemoveActiveTransac
         LOG(("  delay resuming throttled for background tabs"));
         DelayedResumeBackgroundThrottledTransactions();
         return;
     }
 
     if (forActiveTab) {
         // Removing the last transaction for the active tab frees up the unthrottled
         // background tabs transactions.
-        LOG(("  resuming unthrottled for background tabs"));
-        ResumeReadOf(mActiveTransactions[false]);
+        LOG(("  delay resuming unthrottled for background tabs"));
+        DelayedResumeBackgroundThrottledTransactions();
         return;
     }
 
     LOG(("  not resuming anything"));
 }
 
 void
 nsHttpConnectionMgr::MoveActiveTransaction(nsHttpTransaction * aTrans, bool aThrottled)
@@ -3031,47 +3031,67 @@ nsHttpConnectionMgr::ShouldStopReading(n
 {
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
     if (!mThrottlingInhibitsReading || !mThrottleEnabled) {
         return false;
     }
 
     uint64_t tabId = aTrans->TopLevelOuterContentWindowId();
+    bool forActiveTab = tabId == mCurrentTopLevelOuterContentWindowId;
 
     if (mActiveTabTransactionsExist) {
         if (!tabId) {
             // Chrome initiated and unidentified transactions just respect
             // their throttle flag, when something for the active tab is happening.
             return aThrottled;
         }
-        if (tabId != mCurrentTopLevelOuterContentWindowId) {
+        if (!forActiveTab) {
             // This is a background tab request, we want them to always throttle.
             return true;
         }
         if (mActiveTabUnthrottledTransactionsExist) {
             // Unthrottled transactions for the active tab take precedence
             return aThrottled;
         }
         // This is a throttled transaction for the active tab and there are no
         // unthrottled for the active tab, just let go on full fuel.
         return false;
     }
 
+    MOZ_ASSERT(!forActiveTab);
+
+    if (mDelayedResumeReadTimer) {
+        // If this timer exists, background transactions are scheduled to be woken
+        // after a delay.
+        return true;
+    }
+
     if (!mActiveTransactions[false].IsEmpty()) {
         // This means there are unthrottled active transactions for background tabs.
         // If we are here, there can't be any transactions for the active tab.
         // (If there is no transaction for a tab id, there is no entry for it in the hashtable.)
         return aThrottled;
     }
 
     // There are only unthrottled transactions for background tabs: don't throttle.
     return false;
 }
 
+bool nsHttpConnectionMgr::IsConnEntryUnderPressure(nsHttpConnectionInfo *connInfo)
+{
+    nsConnectionEntry *ent = mCT.Get(connInfo->HashKey());
+    MOZ_ASSERT(ent);
+
+    nsTArray<RefPtr<PendingTransactionInfo>> *transactions =
+        ent->mPendingTransactionTable.Get(mCurrentTopLevelOuterContentWindowId);
+
+    return transactions && !transactions->IsEmpty();
+}
+
 bool nsHttpConnectionMgr::IsThrottleTickerNeeded()
 {
     LOG(("nsHttpConnectionMgr::IsThrottleTickerNeeded"));
 
     if (mActiveTabUnthrottledTransactionsExist &&
         mActiveTransactions[false].Count() > 1) {
         LOG(("  there are unthrottled transactions for both active and bck"));
         return true;
@@ -3148,31 +3168,31 @@ void
 nsHttpConnectionMgr::ThrottlerTick()
 {
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
     mThrottlingInhibitsReading = !mThrottlingInhibitsReading;
 
     LOG(("nsHttpConnectionMgr::ThrottlerTick inhibit=%d", mThrottlingInhibitsReading));
 
-    if (!mThrottlingInhibitsReading && !IsThrottleTickerNeeded()) {
+    // If there are only background transactions to be woken after a delay, keep
+    // the ticker so that we woke them only for the resume-for interval and then
+    // throttle them again until the background-resume delay passes.
+    if (!mThrottlingInhibitsReading &&
+        !mDelayedResumeReadTimer &&
+        !IsThrottleTickerNeeded()) {
+        LOG(("  last tick"));
         mThrottleTicker = nullptr;
-    } else {
-        mThrottleTicker = do_CreateInstance("@mozilla.org/timer;1");
     }
 
     if (mThrottlingInhibitsReading) {
         if (mThrottleTicker) {
             mThrottleTicker->Init(this, mThrottleSuspendFor, nsITimer::TYPE_ONE_SHOT);
         }
     } else {
-        // Resume by the ticker happens sooner than delayed resume, no need
-        // for the delayed resume timer
-        CancelDelayedResumeBackgroundThrottledTransactions();
-
         if (mThrottleTicker) {
             mThrottleTicker->Init(this, mThrottleResumeFor, nsITimer::TYPE_ONE_SHOT);
         }
 
         ResumeReadOf(mActiveTransactions[false], true);
         ResumeReadOf(mActiveTransactions[true]);
     }
 }
@@ -3208,25 +3228,25 @@ nsHttpConnectionMgr::CancelDelayedResume
 void
 nsHttpConnectionMgr::ResumeBackgroundThrottledTransactions()
 {
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
     LOG(("nsHttpConnectionMgr::ResumeBackgroundThrottledTransactions"));
     mDelayedResumeReadTimer = nullptr;
 
-    // We can destroy the ticker, since only transactions to resume now
-    // are background throttled.  If 'higher class' transactions have
-    // been added, we don't get here - the ticker has been scheduled
-    // and hence the delayed resume timer canceled.
-    MOZ_ASSERT(mActiveTransactions[false].IsEmpty() &&
-                          !mActiveTabTransactionsExist);
-
-    DestroyThrottleTicker();
-    ResumeReadOf(mActiveTransactions[true], true);
+    if (!IsThrottleTickerNeeded()) {
+        DestroyThrottleTicker();
+    }
+
+    if (!mActiveTransactions[false].IsEmpty()) {
+        ResumeReadOf(mActiveTransactions[false], true);
+    } else {
+        ResumeReadOf(mActiveTransactions[true], true);
+    }
 }
 
 void
 nsHttpConnectionMgr::ResumeReadOf(
     nsClassHashtable<nsUint64HashKey, nsTArray<RefPtr<nsHttpTransaction>>>& hashtable,
     bool excludeForActiveTab)
 {
     for (auto iter = hashtable.Iter(); !iter.Done(); iter.Next()) {
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -221,16 +221,22 @@ public:
     void MoveActiveTransaction(nsHttpTransaction* aTrans, bool aThrottled);
 
     // called by nsHttpTransaction::WriteSegments.  decides whether the transaction
     // should stop reading data based on: the throttling ticker status, overall
     // status of all active transactions regarding active tab and respective
     // throttling state.
     bool ShouldStopReading(nsHttpTransaction* aTrans, bool aThrottled);
 
+    // return true iff the connection has pending transactions for the active tab.
+    // it's mainly used to disallow throttling (stop reading) of a response
+    // belonging to the same conn info to free up a connection ASAP.
+    // NOTE: relatively expensive to call, there are two hashtable lookups.
+    bool IsConnEntryUnderPressure(nsHttpConnectionInfo*);
+
 private:
     virtual ~nsHttpConnectionMgr();
 
     class nsHalfOpenSocket;
     class PendingTransactionInfo;
 
     // nsConnectionEntry
     //
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -815,37 +815,63 @@ nsHttpTransaction::WritePipeSegment(nsIO
     //
     rv = trans->ProcessData(buf, *countWritten, countWritten);
     if (NS_FAILED(rv))
         trans->Close(rv);
 
     return rv; // failure code only stops WriteSegments; it is not propagated.
 }
 
+bool nsHttpTransaction::ShouldStopReading()
+{
+    if (mActivatedAsH2) {
+        // Throttling feature is now disabled for http/2 transactions
+        // because of bug 1367861.  The logic around mActivatedAsH2
+        // will be removed when that is fixed
+        return false;
+    }
+
+    if (!gHttpHandler->ConnMgr()->ShouldStopReading(
+            this, mClassOfService & nsIClassOfService::Throttleable)) {
+        // We are not obligated to throttle
+        return false;
+    }
+
+    if (mContentRead < 16000) {
+        // Let the first bytes go, it may also well be all the content we get
+        LOG(("nsHttpTransaction::ShouldStopReading too few content (%llu) this=%p", mContentRead, this));
+        return false;
+    }
+
+    if (gHttpHandler->ConnMgr()->IsConnEntryUnderPressure(mConnInfo)) {
+        LOG(("nsHttpTransaction::ShouldStopReading entry pressure this=%p", this));
+        // This is expensive to check (two hashtable lookups) but may help
+        // freeing connections for active tab transactions.
+        return false;
+    }
+
+    return true;
+}
+
 nsresult
 nsHttpTransaction::WriteSegments(nsAHttpSegmentWriter *writer,
                                  uint32_t count, uint32_t *countWritten)
 {
     LOG(("nsHttpTransaction::WriteSegments %p", this));
 
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
     if (mTransactionDone) {
         return NS_SUCCEEDED(mStatus) ? NS_BASE_STREAM_CLOSED : mStatus;
     }
 
-    // Throttling feature is now disabled for http/2 transactions
-    // because of bug 1367861.  The logic around mActivatedAsH2
-    // will be removed when that is fixed
-    if (!mActivatedAsH2 &&
-        gHttpHandler->ConnMgr()->ShouldStopReading(this,
-            mClassOfService & nsIClassOfService::Throttleable)) {
-        LOG(("nsHttpTransaction::WriteSegments %p response throttled", this));
+    if (ShouldStopReading()) {
         // Must remember that we have to call ResumeRecv() on our connection when
         // called back by the conn manager to resume reading.
+        LOG(("nsHttpTransaction::WriteSegments %p response throttled", this));
         mReadingStopped = true;
         // This makes the underlaying connection or stream wait for explicit resume.
         // For h1 this means we stop reading from the socket.
         // For h2 this means we stop updating recv window for the stream.
         return NS_BASE_STREAM_WOULD_BLOCK;
     }
 
     mWriter = writer;
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -218,16 +218,20 @@ private:
     // Called right after we parsed the response head.  Checks for connection based
     // authentication schemes in reponse headers for WWW and Proxy authentication.
     // If such is found in any of them, NS_HTTP_STICKY_CONNECTION is set in mCaps.
     // We need the sticky flag be set early to keep the connection from very start
     // of the authentication process.
     void CheckForStickyAuthScheme();
     void CheckForStickyAuthSchemeAt(nsHttpAtom const& header);
 
+    // Called from WriteSegments.  Checks for conditions whether to throttle reading
+    // the content.  When this returns true, WriteSegments returns WOULD_BLOCK.
+    bool ShouldStopReading();
+
 private:
     class UpdateSecurityCallbacks : public Runnable
     {
       public:
         UpdateSecurityCallbacks(nsHttpTransaction* aTrans,
                                 nsIInterfaceRequestor* aCallbacks)
         : mTrans(aTrans), mCallbacks(aCallbacks) {}