bug 869100 complete token bucket a/b test r=honzab

Brli · May 8, 2013 · 03f4f0e · 03f4f0e
1 parent 10e9bd8
commit 03f4f0e
Show file tree

Hide file tree

Showing 8 changed files with 6 additions and 189 deletions.
diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js
@@ -1011,13 +1011,7 @@ 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);

diff --git a/netwerk/base/src/nsLoadGroup.cpp b/netwerk/base/src/nsLoadGroup.cpp
@@ -825,43 +825,7 @@ nsLoadGroup::GetRootLoadGroup(nsILoadGroup * *aRootLoadGroup)
 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;
 }
 

diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1399,17 +1399,6 @@ HttpBaseChannel::SetLoadUnblocked(bool 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
 //-----------------------------------------------------------------------------

diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h
@@ -149,7 +149,6 @@ class HttpBaseChannel : public nsHashPropertyBag
   NS_IMETHOD SetLoadAsBlocking(bool aLoadAsBlocking);
   NS_IMETHOD GetLoadUnblocked(bool *aLoadUnblocked);
   NS_IMETHOD SetLoadUnblocked(bool aLoadUnblocked);
-  NS_IMETHOD GetPacingTelemetryID(uint32_t *aID);
 
   inline void CleanRedirectCacheChainIfNecessary()
   {

diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -189,9 +189,7 @@ nsHttpHandler::nsHttpHandler()
     , mSpdyPingTimeout(PR_SecondsToInterval(8))
     , mConnectTimeout(90000)
     , mParallelSpeculativeConnectLimit(6)
-    , mRequestTokenBucketEnabled(false)
-    , mRequestTokenBucketABTestEnabled(false)
-    , mRequestTokenBucketABTestProfile(0)
+    , mRequestTokenBucketEnabled(true)
     , mRequestTokenBucketMinParallelism(6)
     , mRequestTokenBucketHz(100)
     , mRequestTokenBucketBurst(32)
@@ -777,53 +775,6 @@ nsHttpHandler::MaxSocketCount()
     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)
 {
@@ -1357,20 +1308,6 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
         }
     }
 
-    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))

diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h
@@ -108,10 +108,9 @@ class nsHttpHandler : public nsIHttpProtocolHandler
     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; }
 
@@ -425,8 +424,6 @@ class nsHttpHandler : public nsIHttpProtocolHandler
     // 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

diff --git a/netwerk/protocol/http/nsIHttpChannelInternal.idl b/netwerk/protocol/http/nsIHttpChannelInternal.idl
@@ -34,7 +34,7 @@ interface nsIHttpUpgradeListener : nsISupports
  * 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
 {
     /**
@@ -172,11 +172,4 @@ interface nsIHttpChannelInternal : nsISupports
      */
     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;
 };
diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json
@@ -1968,62 +1968,6 @@
     "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",