From 8ebada0f3411fe33ae68fe47ffdfff10b585d70c Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Wed, 11 Apr 2012 13:49:32 -0400 Subject: [PATCH] bug 743747: fix landing with 603514 r=honzab --- modules/libpref/src/init/all.js | 4 +- netwerk/protocol/http/nsHttpConnection.cpp | 62 +++++++++++++--------- netwerk/protocol/http/nsHttpHandler.cpp | 32 +++++++++-- netwerk/protocol/http/nsHttpHandler.h | 19 ++++++- 4 files changed, 84 insertions(+), 33 deletions(-) diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 6aca6751fc6..f2fc408cab7 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -817,9 +817,11 @@ pref("network.http.pipelining.max-optimistic-requests" , 4); pref("network.http.pipelining.aggressive", false); pref("network.http.pipelining.maxsize" , 300000); +pref("network.http.pipelining.reschedule-on-timeout", true); +pref("network.http.pipelining.reschedule-timeout", 1500); // The read-timeout is a ms timer that causes the transaction to be completely -// restarted without pipelining. Set to 0 to disable. +// restarted without pipelining. pref("network.http.pipelining.read-timeout", 30000); // Prompt for 307 redirects diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index 169522f0b59..c3e4203e2d7 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -957,14 +957,16 @@ nsHttpConnection::ReadTimeoutTick(PRIntervalTime now) if (!mTransaction) return; - // Spdy in the future actually should implement some timeout handling - // using the SPDY ping frame. + // Spdy implements some timeout handling using the SPDY ping frame. if (mSpdySession) { mSpdySession->ReadTimeoutTick(now); return; } - PRIntervalTime delta = PR_IntervalNow() - mLastReadTime; + if (!gHttpHandler->GetPipelineRescheduleOnTimeout()) + return; + + PRIntervalTime delta = now - mLastReadTime; // we replicate some of the checks both here and in OnSocketReadable() as // they will be discovered under different conditions. The ones here @@ -975,28 +977,30 @@ nsHttpConnection::ReadTimeoutTick(PRIntervalTime now) // Right now we only take action if pipelining is involved, but this would // be the place to add general read timeout handling if it is desired. - const PRIntervalTime k1000ms = PR_MillisecondsToInterval(1000); - - if (delta < k1000ms) - return; - PRUint32 pipelineDepth = mTransaction->PipelineDepth(); - // this just reschedules blocked transactions. no transaction - // is aborted completely. - LOG(("cancelling pipeline due to a %ums stall - depth %d\n", - PR_IntervalToMilliseconds(delta), pipelineDepth)); + if (delta >= gHttpHandler->GetPipelineRescheduleTimeout()) { - if (pipelineDepth > 1) { - nsHttpPipeline *pipeline = mTransaction->QueryPipeline(); - NS_ABORT_IF_FALSE(pipeline, "pipelinedepth > 1 without pipeline"); - // code this defensively for the moment and check for null in opt build - if (pipeline) - pipeline->CancelPipeline(NS_ERROR_NET_TIMEOUT); + // this just reschedules blocked transactions. no transaction + // is aborted completely. + LOG(("cancelling pipeline due to a %ums stall - depth %d\n", + PR_IntervalToMilliseconds(delta), pipelineDepth)); + + if (pipelineDepth > 1) { + nsHttpPipeline *pipeline = mTransaction->QueryPipeline(); + NS_ABORT_IF_FALSE(pipeline, "pipelinedepth > 1 without pipeline"); + // code this defensively for the moment and check for null in opt build + // This will reschedule blocked members of the pipeline, but the + // blocking transaction (i.e. response 0) will not be changed. + if (pipeline) { + pipeline->CancelPipeline(NS_ERROR_NET_TIMEOUT); + LOG(("Rescheduling the head of line blocked members of a pipeline " + "because reschedule-timeout idle interval exceeded")); + } + } } - - PRIntervalTime pipelineTimeout = gHttpHandler->GetPipelineTimeout(); - if (!pipelineTimeout || (delta < pipelineTimeout)) + + if (delta < gHttpHandler->GetPipelineTimeout()) return; if (pipelineDepth <= 1 && !mTransaction->PipelinePosition()) @@ -1338,10 +1342,9 @@ nsHttpConnection::OnSocketReadable() else delta = 0; - const PRIntervalTime k400ms = PR_MillisecondsToInterval(400); - const PRIntervalTime k1200ms = PR_MillisecondsToInterval(1200); + static const PRIntervalTime k400ms = PR_MillisecondsToInterval(400); - if (delta > k1200ms) { + if (delta >= (mRtt + gHttpHandler->GetPipelineRescheduleTimeout())) { LOG(("Read delta ms of %u causing slow read major " "event and pipeline cancellation", PR_IntervalToMilliseconds(delta))); @@ -1349,12 +1352,19 @@ nsHttpConnection::OnSocketReadable() gHttpHandler->ConnMgr()->PipelineFeedbackInfo( mConnInfo, nsHttpConnectionMgr::BadSlowReadMajor, this, 0); - if (mTransaction->PipelineDepth() > 1) { + if (gHttpHandler->GetPipelineRescheduleOnTimeout() && + mTransaction->PipelineDepth() > 1) { nsHttpPipeline *pipeline = mTransaction->QueryPipeline(); NS_ABORT_IF_FALSE(pipeline, "pipelinedepth > 1 without pipeline"); // code this defensively for the moment and check for null - if (pipeline) + // This will reschedule blocked members of the pipeline, but the + // blocking transaction (i.e. response 0) will not be changed. + if (pipeline) { pipeline->CancelPipeline(NS_ERROR_NET_TIMEOUT); + LOG(("Rescheduling the head of line blocked members of a " + "pipeline because reschedule-timeout idle interval " + "exceeded")); + } } } else if (delta > k400ms) { diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 18ae4d86333..56bca0ec04d 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -189,6 +189,8 @@ nsHttpHandler::nsHttpHandler() , mMaxOptimisticPipelinedRequests(4) , mPipelineAggressive(false) , mMaxPipelineObjectSize(300000) + , mPipelineRescheduleOnTimeout(true) + , mPipelineRescheduleTimeout(PR_MillisecondsToInterval(1500)) , mPipelineReadTimeout(PR_MillisecondsToInterval(30000)) , mRedirectionLimit(10) , mPhishyUserPassLength(1) @@ -1055,14 +1057,34 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref) } } + // Determines whether or not to actually reschedule after the + // reschedule-timeout has expired + if (PREF_CHANGED(HTTP_PREF("pipelining.reschedule-on-timeout"))) { + rv = prefs->GetBoolPref(HTTP_PREF("pipelining.reschedule-on-timeout"), + &cVar); + if (NS_SUCCEEDED(rv)) + mPipelineRescheduleOnTimeout = cVar; + } + + // The amount of time head of line blocking is allowed (in ms) + // before the blocked transactions are moved to another pipeline + if (PREF_CHANGED(HTTP_PREF("pipelining.reschedule-timeout"))) { + rv = prefs->GetIntPref(HTTP_PREF("pipelining.reschedule-timeout"), + &val); + if (NS_SUCCEEDED(rv)) { + mPipelineRescheduleTimeout = + PR_MillisecondsToInterval((PRUint16) clamped(val, 500, 0xffff)); + } + } + + // The amount of time a pipelined transaction is allowed to wait before + // being canceled and retried in a non-pipeline connection if (PREF_CHANGED(HTTP_PREF("pipelining.read-timeout"))) { rv = prefs->GetIntPref(HTTP_PREF("pipelining.read-timeout"), &val); if (NS_SUCCEEDED(rv)) { - if (!val) - mPipelineReadTimeout = 0; - else - mPipelineReadTimeout = - PR_MillisecondsToInterval(clamped(val, 500, 0xffff)); + mPipelineReadTimeout = + PR_MillisecondsToInterval((PRUint16) clamped(val, 5000, + 0xffff)); } } diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 1489fb69dde..05bcbf232ca 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -237,6 +237,22 @@ public: { *outVal = mMaxPipelineObjectSize; } + + bool GetPipelineEnabled() + { + return mCapabilities & NS_HTTP_ALLOW_PIPELINING; + } + + bool GetPipelineRescheduleOnTimeout() + { + return mPipelineRescheduleOnTimeout; + } + + PRIntervalTime GetPipelineRescheduleTimeout() + { + return mPipelineRescheduleTimeout; + } + PRIntervalTime GetPipelineTimeout() { return mPipelineReadTimeout; } private: @@ -299,7 +315,8 @@ private: PRUint16 mMaxOptimisticPipelinedRequests; bool mPipelineAggressive; PRInt64 mMaxPipelineObjectSize; - + bool mPipelineRescheduleOnTimeout; + PRIntervalTime mPipelineRescheduleTimeout; PRIntervalTime mPipelineReadTimeout; PRUint8 mRedirectionLimit;