From 002027aa6c21822e47f4eb2e51c10e2ae353f6fe Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Mon, 27 Feb 2012 10:32:09 -0500 Subject: [PATCH] bug 729736 - runtime experiment with different spdy ping thresholds r=honzab --- modules/libpref/src/init/all.js | 5 ++ .../base/src/nsSocketTransportService2.cpp | 3 + netwerk/protocol/http/SpdySession.cpp | 70 +++++++++++++++++-- netwerk/protocol/http/SpdySession.h | 4 ++ netwerk/protocol/http/nsHttpHandler.cpp | 50 +++++++++++++ netwerk/protocol/http/nsHttpHandler.h | 11 +++ .../telemetry/TelemetryHistograms.h | 2 + 7 files changed, 139 insertions(+), 6 deletions(-) diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 5da86bd7536..59d82be91c4 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -678,6 +678,11 @@ pref("security.fileuri.strict_origin_policy", true); // prevents necko connecting to ports 1-5 unless the protocol // overrides. +// Allow necko to do A/B testing. Will generally only happen if +// telemetry is also enabled as otherwise there is no way to report +// the results +pref("network.allow-experiments", true); + // Default action for unlisted external protocol handlers pref("network.protocol-handler.external-default", true); // OK to load pref("network.protocol-handler.warn-external-default", true); // warn before load diff --git a/netwerk/base/src/nsSocketTransportService2.cpp b/netwerk/base/src/nsSocketTransportService2.cpp index 87913163bfc..68574c37f4e 100644 --- a/netwerk/base/src/nsSocketTransportService2.cpp +++ b/netwerk/base/src/nsSocketTransportService2.cpp @@ -632,6 +632,9 @@ nsSocketTransportService::Run() nsCOMPtr threadInt = do_QueryInterface(thread); threadInt->SetObserver(this); + // make sure the pseudo random number generator is seeded on this thread + srand(PR_Now()); + for (;;) { bool pendingEvents = false; thread->HasPendingEvents(&pendingEvents); diff --git a/netwerk/protocol/http/SpdySession.cpp b/netwerk/protocol/http/SpdySession.cpp index 8a893c72350..9a816dfaea7 100644 --- a/netwerk/protocol/http/SpdySession.cpp +++ b/netwerk/protocol/http/SpdySession.cpp @@ -93,7 +93,8 @@ SpdySession::SpdySession(nsAHttpTransaction *aHttpTransaction, mOutputQueueSent(0), mLastReadEpoch(PR_IntervalNow()), mPingSentEpoch(0), - mNextPingID(1) + mNextPingID(1), + mPingThresholdExperiment(false) { NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); @@ -111,6 +112,41 @@ SpdySession::SpdySession(nsAHttpTransaction *aHttpTransaction, mSendingChunkSize = gHttpHandler->SpdySendingChunkSize(); AddStream(aHttpTransaction, firstPriority); mLastDataReadEpoch = mLastReadEpoch; + + DeterminePingThreshold(); +} + +void +SpdySession::DeterminePingThreshold() +{ + mPingThreshold = gHttpHandler->SpdyPingThreshold(); + + if (!mPingThreshold || !gHttpHandler->AllowExperiments()) + return; + + PRUint32 randomVal = gHttpHandler->Get32BitsOfPseudoRandom(); + + // Use the lower 10 bits to select 1 in 1024 sessions for the + // ping threshold experiment. Somewhat less than that will actually be + // used because random values greater than the total http idle timeout + // for the session are discarded. + if ((randomVal & 0x3ff) != 1) // lottery + return; + + randomVal = randomVal >> 10; // those bits are used up + + // This session has been selected - use a random ping threshold of 10 + + // a random number from 0 to 255, based on the next 8 bits of the + // random buffer + PRIntervalTime randomThreshold = + PR_SecondsToInterval((randomVal & 0xff) + 10); + if (randomThreshold > gHttpHandler->IdleTimeout()) + return; + + mPingThreshold = randomThreshold; + mPingThresholdExperiment = true; + LOG3(("SpdySession %p Ping Threshold Experimental Selection : %dsec\n", + this, PR_IntervalToSeconds(mPingThreshold))); } PLDHashOperator @@ -228,16 +264,16 @@ SpdySession::ReadTimeoutTick(PRIntervalTime now) NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); NS_ABORT_IF_FALSE(mNextPingID & 1, "Ping Counter Not Odd"); - PRIntervalTime threshold = gHttpHandler->SpdyPingThreshold(); - if (!threshold) + if (!mPingThreshold) return; LOG(("SpdySession::ReadTimeoutTick %p delta since last read %ds\n", this, PR_IntervalToSeconds(now - mLastReadEpoch))); - if ((now - mLastReadEpoch) < threshold) { + if ((now - mLastReadEpoch) < mPingThreshold) { // recent activity means ping is not an issue - mPingSentEpoch = 0; + if (mPingSentEpoch) + ClearPing(true); return; } @@ -246,6 +282,7 @@ SpdySession::ReadTimeoutTick(PRIntervalTime now) if ((now - mPingSentEpoch) >= gHttpHandler->SpdyPingTimeout()) { LOG(("SpdySession::ReadTimeoutTick %p Ping Timer Exhaustion\n", this)); + ClearPing(false); Close(NS_ERROR_NET_TIMEOUT); } return; @@ -273,6 +310,27 @@ SpdySession::ReadTimeoutTick(PRIntervalTime now) } } +void +SpdySession::ClearPing(bool pingOK) +{ + mPingSentEpoch = 0; + + if (mPingThresholdExperiment) { + LOG3(("SpdySession::ClearPing %p mPingThresholdExperiment %dsec %s\n", + this, PR_IntervalToSeconds(mPingThreshold), + pingOK ? "pass" :"fail")); + + if (pingOK) + Telemetry::Accumulate(Telemetry::SPDY_PING_EXPERIMENT_PASS, + PR_IntervalToSeconds(mPingThreshold)); + else + Telemetry::Accumulate(Telemetry::SPDY_PING_EXPERIMENT_FAIL, + PR_IntervalToSeconds(mPingThreshold)); + mPingThreshold = gHttpHandler->SpdyPingThreshold(); + mPingThresholdExperiment = false; + } +} + PRUint32 SpdySession::RegisterStreamID(SpdyStream *stream) { @@ -1187,7 +1245,7 @@ SpdySession::HandlePing(SpdySession *self) if (pingID & 0x01) { // presumably a reply to our timeout ping - self->mPingSentEpoch = 0; + self->ClearPing(true); } else { // Servers initiate even numbered pings, go ahead and echo it back diff --git a/netwerk/protocol/http/SpdySession.h b/netwerk/protocol/http/SpdySession.h index 0249958223d..d1e9b9e729b 100644 --- a/netwerk/protocol/http/SpdySession.h +++ b/netwerk/protocol/http/SpdySession.h @@ -188,6 +188,7 @@ private: PROCESSING_CONTROL_RST_STREAM }; + void DeterminePingThreshold(); nsresult HandleSynReplyForValidStream(); PRUint32 GetWriteQueueSize(); void ChangeDownstreamState(enum stateType); @@ -198,6 +199,7 @@ private: nsresult ConvertHeaders(nsDependentCSubstring &, nsDependentCSubstring &); void GeneratePing(PRUint32); + void ClearPing(bool); void GenerateRstStream(PRUint32, PRUint32); void GenerateGoAway(); void CleanupStream(SpdyStream *, nsresult, rstReason); @@ -345,10 +347,12 @@ private: PRUint32 mOutputQueueSent; nsAutoArrayPtr mOutputQueueBuffer; + PRIntervalTime mPingThreshold; PRIntervalTime mLastReadEpoch; // used for ping timeouts PRIntervalTime mLastDataReadEpoch; // used for IdleTime() PRIntervalTime mPingSentEpoch; PRUint32 mNextPingID; + bool mPingThresholdExperiment; }; }} // namespace mozilla::net diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index d22762d5fec..dcdf729368b 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -128,6 +128,8 @@ static NS_DEFINE_CID(kSocketProviderServiceCID, NS_SOCKETPROVIDERSERVICE_CID); #define NETWORK_ENABLEIDN "network.enableIDN" #define BROWSER_PREF_PREFIX "browser.cache." #define DONOTTRACK_HEADER_ENABLED "privacy.donottrackheader.enabled" +#define TELEMETRY_ENABLED "toolkit.telemetry.enabled" +#define ALLOW_EXPERIMENTS "network.allow-experiments" #define UA_PREF(_pref) UA_PREF_PREFIX _pref #define HTTP_PREF(_pref) HTTP_PREF_PREFIX _pref @@ -200,6 +202,8 @@ nsHttpHandler::nsHttpHandler() , mSendSecureXSiteReferrer(true) , mEnablePersistentHttpsCaching(false) , mDoNotTrackEnabled(false) + , mTelemetryEnabled(false) + , mAllowExperiments(true) , mEnableSpdy(false) , mCoalesceSpdy(true) , mUseAlternateProtocol(false) @@ -268,6 +272,7 @@ nsHttpHandler::Init() prefBranch->AddObserver(NETWORK_ENABLEIDN, this, true); prefBranch->AddObserver(BROWSER_PREF("disk_cache_ssl"), this, true); prefBranch->AddObserver(DONOTTRACK_HEADER_ENABLED, this, true); + prefBranch->AddObserver(TELEMETRY_ENABLED, this, true); PrefsChanged(prefBranch, nsnull); } @@ -536,6 +541,27 @@ nsHttpHandler::GetIOService(nsIIOService** result) return NS_OK; } +PRUint32 +nsHttpHandler::Get32BitsOfPseudoRandom() +{ + // only confirm rand seeding on socket thread + NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread"); + + // rand() provides different amounts of PRNG on different platforms. + // 15 or 31 bits are common amounts. + + PR_STATIC_ASSERT(RAND_MAX >= 0xfff); + +#if RAND_MAX < 0xffffU + return ((PRUint16) rand() << 20) | + (((PRUint16) rand() & 0xfff) << 8) | + ((PRUint16) rand() & 0xff); +#elif RAND_MAX < 0xffffffffU + return ((PRUint16) rand() << 16) | ((PRUint16) rand() & 0xffff); +#else + return (PRUint32) rand(); +#endif +} void nsHttpHandler::NotifyObservers(nsIHttpChannel *chan, const char *event) @@ -1187,6 +1213,30 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref) } } + // + // Telemetry + // + + if (PREF_CHANGED(TELEMETRY_ENABLED)) { + cVar = false; + rv = prefs->GetBoolPref(TELEMETRY_ENABLED, &cVar); + if (NS_SUCCEEDED(rv)) { + mTelemetryEnabled = cVar; + } + } + + // + // network.allow-experiments + // + + if (PREF_CHANGED(ALLOW_EXPERIMENTS)) { + cVar = true; + rv = prefs->GetBoolPref(ALLOW_EXPERIMENTS, &cVar); + if (NS_SUCCEEDED(rv)) { + mAllowExperiments = cVar; + } + } + #undef PREF_CHANGED #undef MULTI_PREF_CHANGED } diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index 09def7be5f1..b1d02acb9e2 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -112,6 +112,8 @@ public: PRUint32 MaxSocketCount(); bool IsPersistentHttpsCachingEnabled() { return mEnablePersistentHttpsCaching; } + bool IsTelemetryEnabled() { return mTelemetryEnabled; } + bool AllowExperiments() { return mTelemetryEnabled && mAllowExperiments; } bool IsSpdyEnabled() { return mEnableSpdy; } bool CoalesceSpdy() { return mCoalesceSpdy; } @@ -191,6 +193,9 @@ public: nsICookieService * GetCookieService(); // not addrefed nsIStrictTransportSecurityService * GetSTSService(); + // callable from socket thread only + PRUint32 Get32BitsOfPseudoRandom(); + // Called by the channel before writing a request void OnModifyRequest(nsIHttpChannel *chan) { @@ -343,6 +348,12 @@ private: // For broadcasting the preference to not be tracked bool mDoNotTrackEnabled; + // Whether telemetry is reported or not + bool mTelemetryEnabled; + + // The value of network.allow-experiments + bool mAllowExperiments; + // Try to use SPDY features instead of HTTP/1.1 over SSL bool mEnableSpdy; bool mCoalesceSpdy; diff --git a/toolkit/components/telemetry/TelemetryHistograms.h b/toolkit/components/telemetry/TelemetryHistograms.h index ac21c30d8fe..60ca7c35930 100644 --- a/toolkit/components/telemetry/TelemetryHistograms.h +++ b/toolkit/components/telemetry/TelemetryHistograms.h @@ -200,6 +200,8 @@ HISTOGRAM(SPDY_SYN_REPLY_RATIO, 1, 99, 20, LINEAR, "SPDY: SYN Reply Header Rati HISTOGRAM(SPDY_NPN_CONNECT, 0, 1, 2, BOOLEAN, "SPDY: NPN Negotiated") HISTOGRAM(SPDY_NPN_JOIN, 0, 1, 2, BOOLEAN, "SPDY: Coalesce Succeeded") HISTOGRAM(SPDY_KBREAD_PER_CONN, 1, 3000, 50, EXPONENTIAL, "SPDY: KB read per connection") +HISTOGRAM(SPDY_PING_EXPERIMENT_PASS, 10, 355, 64, LINEAR, "SPDY: Ping Interval Passed") +HISTOGRAM(SPDY_PING_EXPERIMENT_FAIL, 10, 355, 64, LINEAR, "SPDY: Ping Interval Failed") HISTOGRAM(SPDY_SETTINGS_UL_BW, 1, 10000, 100, EXPONENTIAL, "SPDY: Settings Upload Bandwidth") HISTOGRAM(SPDY_SETTINGS_DL_BW, 1, 10000, 100, EXPONENTIAL, "SPDY: Settings Download Bandwidth")