bug 869100 complete token bucket a/b test r=honzab
authorPatrick McManus <mcmanus@ducksong.com>
Wed, 08 May 2013 11:43:31 -0400
changeset 131257 5566cebc8766b41174d7827361e34950ba717d4e
parent 131256 2d98636bc27bb9b5d7a06543279e6327ef31b891
child 131258 db7759b0338925354e5c112c33e6a75f96d15d99
push id27757
push usermcmanus@ducksong.com
push dateWed, 08 May 2013 15:44:01 +0000
treeherdermozilla-inbound@5566cebc8766 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs869100
milestone23.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 869100 complete token bucket a/b test r=honzab
modules/libpref/src/init/all.js
netwerk/base/src/nsLoadGroup.cpp
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
netwerk/protocol/http/nsIHttpChannelInternal.idl
toolkit/components/telemetry/Histograms.json
--- a/modules/libpref/src/init/all.js
+++ b/modules/libpref/src/init/all.js
@@ -1006,23 +1006,17 @@ pref("network.http.spdy.coalesce-hostnam
 pref("network.http.spdy.use-alternate-protocol", true);
 pref("network.http.spdy.persistent-settings", false);
 pref("network.http.spdy.ping-threshold", 58);
 pref("network.http.spdy.ping-timeout", 8);
 pref("network.http.spdy.send-buffer-size", 131072);
 
 pref("network.http.diagnostics", false);
 
-#ifdef RELEASE_BUILD
-pref("network.http.pacing.requests.enabled", false);
-pref("network.http.pacing.requests.abtest", false);
-#else
 pref("network.http.pacing.requests.enabled", true);
-pref("network.http.pacing.requests.abtest", true);
-#endif
 pref("network.http.pacing.requests.min-parallelism", 6);
 pref("network.http.pacing.requests.hz", 100);
 pref("network.http.pacing.requests.burst", 32);
 
 // default values for FTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,
 // Section 4.8 "High-Throughput Data Service Class", and 80 (0x50, or AF22)
 // per Section 4.7 "Low-Latency Data Service Class".
--- a/netwerk/base/src/nsLoadGroup.cpp
+++ b/netwerk/base/src/nsLoadGroup.cpp
@@ -820,53 +820,17 @@ nsLoadGroup::GetRootLoadGroup(nsILoadGro
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsPILoadGroupInternal methods:
 
 NS_IMETHODIMP
 nsLoadGroup::OnEndPageLoad(nsIChannel *aDefaultChannel)
 {
-    // On page load of a page with at least 32 resources we want to record
-    // telemetry on which rate pacing algorithm was used to load the page and
-    // the page load time.
-    uint32_t requests = mTimedNonCachedRequestsUntilOnEndPageLoad;
-    mTimedNonCachedRequestsUntilOnEndPageLoad = 0;
-
-    nsCOMPtr<nsITimedChannel> timedChannel =
-        do_QueryInterface(aDefaultChannel);
-    if (!timedChannel)
-        return NS_OK;
-
-    nsCOMPtr<nsIHttpChannelInternal> internalHttpChannel =
-        do_QueryInterface(timedChannel);
-    if (!internalHttpChannel)
-        return NS_OK;
-
-    TimeStamp cacheReadStart;
-    TimeStamp asyncOpen;
-    uint32_t telemetryID;
-    nsresult rv = timedChannel->GetCacheReadStart(&cacheReadStart);
-    if (NS_SUCCEEDED(rv))
-        rv = timedChannel->GetAsyncOpen(&asyncOpen);
-    if (NS_SUCCEEDED(rv))
-        rv = internalHttpChannel->GetPacingTelemetryID(&telemetryID);
-    if (NS_FAILED(rv))
-        return NS_OK;
-
-    // Nothing to do if we don't have a start time or this was
-    // from the cache or we don't know what profile was used
-    if (asyncOpen.IsNull() || !cacheReadStart.IsNull() || !telemetryID)
-        return NS_OK;
-
-    if (requests < 32)
-        return NS_OK;
-
-    Telemetry::AccumulateTimeDelta(static_cast<Telemetry::ID>(telemetryID),
-                                   asyncOpen);
+    // for the moment, nothing to do here.
     return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsISupportsPriority methods:
 
 NS_IMETHODIMP
 nsLoadGroup::GetPriority(int32_t *aValue)
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1394,27 +1394,16 @@ HttpBaseChannel::GetLoadUnblocked(bool *
 
 NS_IMETHODIMP
 HttpBaseChannel::SetLoadUnblocked(bool aLoadUnblocked)
 {
   mLoadUnblocked = aLoadUnblocked;
   return NS_OK;
 }
 
-NS_IMETHODIMP
-HttpBaseChannel::GetPacingTelemetryID(uint32_t *aID)
-{
-  *aID = 0;
-  // Don't record pacing algorithm for spdy because it is effectively
-  // bypassed by the protocol mux
-  if (!mResponseHead || !mResponseHead->PeekHeader(nsHttp::X_Firefox_Spdy))
-    *aID = gHttpHandler->PacingTelemetryID();
-  return NS_OK;
-}
-
 //-----------------------------------------------------------------------------
 // HttpBaseChannel::nsISupportsPriority
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 HttpBaseChannel::GetPriority(int32_t *value)
 {
   *value = mPriority;
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -144,17 +144,16 @@ public:
   NS_IMETHOD GetRemoteAddress(nsACString& addr);
   NS_IMETHOD GetRemotePort(int32_t* port);
   NS_IMETHOD GetAllowSpdy(bool *aAllowSpdy);
   NS_IMETHOD SetAllowSpdy(bool aAllowSpdy);
   NS_IMETHOD GetLoadAsBlocking(bool *aLoadAsBlocking);
   NS_IMETHOD SetLoadAsBlocking(bool aLoadAsBlocking);
   NS_IMETHOD GetLoadUnblocked(bool *aLoadUnblocked);
   NS_IMETHOD SetLoadUnblocked(bool aLoadUnblocked);
-  NS_IMETHOD GetPacingTelemetryID(uint32_t *aID);
   
   inline void CleanRedirectCacheChainIfNecessary()
   {
       mRedirectedCachekeys = nullptr;
   }
   NS_IMETHOD HTTPUpgrade(const nsACString & aProtocolName,
                          nsIHttpUpgradeListener *aListener); 
 
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -184,19 +184,17 @@ nsHttpHandler::nsHttpHandler()
     , mUseAlternateProtocol(false)
     , mSpdyPersistentSettings(false)
     , mSpdySendingChunkSize(ASpdySession::kSendingChunkSize)
     , mSpdySendBufferSize(ASpdySession::kTCPSendBufferSize)
     , mSpdyPingThreshold(PR_SecondsToInterval(58))
     , mSpdyPingTimeout(PR_SecondsToInterval(8))
     , mConnectTimeout(90000)
     , mParallelSpeculativeConnectLimit(6)
-    , mRequestTokenBucketEnabled(false)
-    , mRequestTokenBucketABTestEnabled(false)
-    , mRequestTokenBucketABTestProfile(0)
+    , mRequestTokenBucketEnabled(true)
     , mRequestTokenBucketMinParallelism(6)
     , mRequestTokenBucketHz(100)
     , mRequestTokenBucketBurst(32)
     , mCritialRequestPrioritization(true)
 {
 #if defined(PR_LOGGING)
     gHttpLog = PR_NewLogModule("nsHttp");
 #endif
@@ -772,63 +770,16 @@ nsHttpHandler::MaxSocketCount()
     if (maxCount <= 8)
         maxCount = 1;
     else
         maxCount -= 8;
 
     return maxCount;
 }
 
-// Different profiles for when the Token Bucket ABTest is enabled
-static const uint32_t sNumberTokenBucketProfiles = 7;
-static const uint32_t sTokenBucketProfiles[sNumberTokenBucketProfiles][4] = {
-    // burst, hz, min-parallelism
-    { 32, 100, 6, Telemetry::HTTP_PLT_RATE_PACING_0 }, // balanced
-    { 16, 100, 6, Telemetry::HTTP_PLT_RATE_PACING_1 }, // start earlier
-    { 32, 200, 6, Telemetry::HTTP_PLT_RATE_PACING_2 }, // run faster
-    { 32, 50, 6, Telemetry::HTTP_PLT_RATE_PACING_3 },  // run slower
-    { 32, 1, 8, Telemetry::HTTP_PLT_RATE_PACING_4 },   // allow only min-parallelism
-    { 32, 1, 16, Telemetry::HTTP_PLT_RATE_PACING_5 },  // allow only min-parallelism (larger)
-    { 1000, 1000, 1000, Telemetry::HTTP_PLT_RATE_PACING_6 }, // unlimited
-};
-
-uint32_t
-nsHttpHandler::RequestTokenBucketBurst()
-{
-    return AllowExperiments() && mRequestTokenBucketABTestEnabled ?
-        sTokenBucketProfiles[mRequestTokenBucketABTestProfile][0] :
-        mRequestTokenBucketBurst;
-}
-
-uint32_t
-nsHttpHandler::RequestTokenBucketHz()
-{
-    return AllowExperiments() && mRequestTokenBucketABTestEnabled ?
-        sTokenBucketProfiles[mRequestTokenBucketABTestProfile][1] :
-        mRequestTokenBucketHz;
-}
-
-uint16_t
-nsHttpHandler::RequestTokenBucketMinParallelism()
-{
-    uint32_t rv =
-        AllowExperiments() && mRequestTokenBucketABTestEnabled ?
-        sTokenBucketProfiles[mRequestTokenBucketABTestProfile][2] :
-        mRequestTokenBucketMinParallelism;
-    return static_cast<uint16_t>(rv);
-}
-
-uint32_t
-nsHttpHandler::PacingTelemetryID()
-{
-    if (!mRequestTokenBucketEnabled || !mRequestTokenBucketABTestEnabled)
-        return 0;
-    return sTokenBucketProfiles[mRequestTokenBucketABTestProfile][3];
-}
-
 void
 nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
 {
     nsresult rv = NS_OK;
     int32_t val;
 
     LOG(("nsHttpHandler::PrefsChanged [pref=%s]\n", pref));
 
@@ -1352,30 +1303,16 @@ nsHttpHandler::PrefsChanged(nsIPrefBranc
         rv = prefs->GetBoolPref(HTTP_PREF("pacing.requests.enabled"),
                                 &cVar);
         if (NS_SUCCEEDED(rv)){
             requestTokenBucketUpdated = true;
             mRequestTokenBucketEnabled = cVar;
         }
     }
 
-    if (PREF_CHANGED(HTTP_PREF("pacing.requests.abtest"))) {
-        rv = prefs->GetBoolPref(HTTP_PREF("pacing.requests.abtest"),
-                                &cVar);
-        if (NS_SUCCEEDED(rv)) {
-            mRequestTokenBucketABTestEnabled = cVar;
-            requestTokenBucketUpdated = true;
-            if (mRequestTokenBucketABTestEnabled) {
-                // just taking the remainder is not perfectly uniform but it doesn't
-                // matter here.
-                mRequestTokenBucketABTestProfile = rand() % sNumberTokenBucketProfiles;
-            }
-        }
-    }
-
     if (PREF_CHANGED(HTTP_PREF("pacing.requests.min-parallelism"))) {
         rv = prefs->GetIntPref(HTTP_PREF("pacing.requests.min-parallelism"), &val);
         if (NS_SUCCEEDED(rv))
             mRequestTokenBucketMinParallelism = static_cast<uint16_t>(clamped(val, 1, 1024));
     }
     if (PREF_CHANGED(HTTP_PREF("pacing.requests.hz"))) {
         rv = prefs->GetIntPref(HTTP_PREF("pacing.requests.hz"), &val);
         if (NS_SUCCEEDED(rv)) {
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -103,20 +103,19 @@ public:
     uint32_t       SpdySendBufferSize()      { return mSpdySendBufferSize; }
     PRIntervalTime SpdyPingThreshold() { return mSpdyPingThreshold; }
     PRIntervalTime SpdyPingTimeout() { return mSpdyPingTimeout; }
     uint32_t       ConnectTimeout()  { return mConnectTimeout; }
     uint32_t       ParallelSpeculativeConnectLimit() { return mParallelSpeculativeConnectLimit; }
     bool           CritialRequestPrioritization() { return mCritialRequestPrioritization; }
 
     bool           UseRequestTokenBucket() { return mRequestTokenBucketEnabled; }
-    uint16_t       RequestTokenBucketMinParallelism();
-    uint32_t       RequestTokenBucketHz();
-    uint32_t       RequestTokenBucketBurst();
-    uint32_t       PacingTelemetryID();
+    uint16_t       RequestTokenBucketMinParallelism() { return mRequestTokenBucketMinParallelism; }
+    uint32_t       RequestTokenBucketHz() { return mRequestTokenBucketHz; }
+    uint32_t       RequestTokenBucketBurst() {return mRequestTokenBucketBurst; }
 
     bool           PromptTempRedirect()      { return mPromptTempRedirect; }
 
     nsHttpAuthCache     *AuthCache(bool aPrivate) {
         return aPrivate ? &mPrivateAuthCache : &mAuthCache;
     }
     nsHttpConnectionMgr *ConnMgr()   { return mConnMgr; }
 
@@ -420,18 +419,16 @@ private:
 
     // The maximum number of current global half open sockets allowable
     // when starting a new speculative connection.
     uint32_t       mParallelSpeculativeConnectLimit;
 
     // For Rate Pacing of HTTP/1 requests through a netwerk/base/src/EventTokenBucket
     // Active requests <= *MinParallelism are not subject to the rate pacing
     bool           mRequestTokenBucketEnabled;
-    bool           mRequestTokenBucketABTestEnabled;
-    uint32_t       mRequestTokenBucketABTestProfile;
     uint16_t       mRequestTokenBucketMinParallelism;
     uint32_t       mRequestTokenBucketHz;  // EventTokenBucket HZ
     uint32_t       mRequestTokenBucketBurst; // EventTokenBucket Burst
 
     // Whether or not to block requests for non head js/css items (e.g. media)
     // while those elements load.
     bool           mCritialRequestPrioritization;
 
--- a/netwerk/protocol/http/nsIHttpChannelInternal.idl
+++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl
@@ -29,17 +29,17 @@ interface nsIHttpUpgradeListener : nsISu
                               in nsIAsyncOutputStream aSocketOut);
 };
 
 /**
  * Dumping ground for http.  This interface will never be frozen.  If you are
  * using any feature exposed by this interface, be aware that this interface
  * will change and you will be broken.  You have been warned.
  */
-[scriptable, uuid(68074a18-49a8-44d8-852c-320ad4b1d308)]
+[scriptable, uuid(2cd7f6a6-63f3-4bd6-a0f5-6e3d6dcff81b)]
 interface nsIHttpChannelInternal : nsISupports
 {
     /**
      * An http channel can own a reference to the document URI
      */
     attribute nsIURI documentURI;
 
     /**
@@ -167,16 +167,9 @@ interface nsIHttpChannelInternal : nsISu
 
     /**
      * If set, this channel will load in parallel with the rest of the load
      * group even if a blocking subset of the group would normally be given
      * exclusivity. Default false.
      */
     attribute boolean loadUnblocked;
 
-    /**
-     * bug 819734 we implement experimental rate pacing strategies for pages
-     * with large numbers of subresources. Several experimental profiles are
-     * available and this attribute corresponds to the profile used for this
-     * channel. (or at least the one active when it is queried.)
-     */
-     readonly attribute uint32_t pacingTelemetryID;
 };
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -1963,72 +1963,16 @@
   },
   "NETWORK_DISK_CACHE_STREAMIO_CLOSE_MAIN_THREAD": {
     "kind": "exponential",
     "high": "10000",
     "n_buckets": 10,
     "extended_statistics_ok": true,
     "description": "Time spent in nsDiskCacheStreamIO::Close() on the main thread (ms)"
   },
-  "HTTP_PLT_RATE_PACING_0": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 0 (burst=32, hz=100, par=6)"
-  },
-  "HTTP_PLT_RATE_PACING_1": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 1 (burst=16, hz=100, par=6)"
-  },
-  "HTTP_PLT_RATE_PACING_2": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 2 (burst=32, hz=200, par=6)"
-  },
-  "HTTP_PLT_RATE_PACING_3": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 3 (burst=32, hz=50, par=6)"
-  },
-  "HTTP_PLT_RATE_PACING_4": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 4 (burst=32, hz=1, par=8)"
-  },
-  "HTTP_PLT_RATE_PACING_5": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 5 (burst=32, hz=1, par=16)"
-  },
-  "HTTP_PLT_RATE_PACING_6": {
-    "kind": "exponential",
-    "low": 100,
-    "high": "30000",
-    "n_buckets": 100,
-    "extended_statistics_ok": true,
-    "description": "HTTP: Total page load time (ms) with rate pacing algorithm 6 (burst=1000, hz=1000, par=1000)"
-  },
   "IDLE_NOTIFY_BACK_MS": {
     "kind": "exponential",
     "high": "5000",
     "n_buckets": 10,
     "extended_statistics_ok": true,
     "description": "Time spent checking for and notifying listeners that the user is back (ms)"
   },
   "IDLE_NOTIFY_BACK_LISTENERS": {