bug 1189645 - remove spdy telem r=hurley
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 31 Jul 2015 04:07:07 -0400
changeset 287759 e293bbf8223c245e0948dee9ad929f87c1061b5f
parent 287758 0f3c55093831edcb5ce54b083a1fbeccc39324a7
child 287760 4ecae4a2b9ca9c5b0d804d04943490ac7f743f37
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershurley
bugs1189645
milestone42.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 1189645 - remove spdy telem r=hurley
netwerk/protocol/http/SpdySession31.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
toolkit/components/telemetry/Histograms.json
--- a/netwerk/protocol/http/SpdySession31.cpp
+++ b/netwerk/protocol/http/SpdySession31.cpp
@@ -793,30 +793,16 @@ SpdySession31::GenerateSettings()
   if (!gHttpHandler->AllowPush()) {
     // announcing that we accept 0 incoming streams is done to
     // disable server push
     packet[15 + 8 * numberOfEntries] = SETTINGS_TYPE_MAX_CONCURRENT;
     // The value portion of the setting pair is already initialized to 0
     numberOfEntries++;
   }
 
-  nsRefPtr<nsHttpConnectionInfo> ci;
-  uint32_t cwnd = 0;
-  GetConnectionInfo(getter_AddRefs(ci));
-  if (ci)
-    cwnd = gHttpHandler->ConnMgr()->GetSpdyCWNDSetting(ci);
-  if (cwnd) {
-    packet[12 + 8 * numberOfEntries] = PERSISTED_VALUE;
-    packet[15 + 8 * numberOfEntries] = SETTINGS_TYPE_CWND;
-    LOG(("SpdySession31::GenerateSettings %p sending CWND %u\n", this, cwnd));
-    cwnd = PR_htonl(cwnd);
-    memcpy(packet + 16 + 8 * numberOfEntries, &cwnd, 4);
-    numberOfEntries++;
-  }
-
   // Advertise the Push RWIN and on each client SYN_STREAM pipeline
   // a window update with it in order to use larger initial windows with pulled
   // streams.
   packet[15 + 8 * numberOfEntries] = SETTINGS_TYPE_INITIAL_WINDOW;
   uint32_t rwin = PR_htonl(mPushAllowance);
   memcpy(packet + 16 + 8 * numberOfEntries, &rwin, 4);
   numberOfEntries++;
 
@@ -1440,51 +1426,22 @@ SpdySession31::HandleSettings(SpdySessio
     uint32_t flags = setting[0];
     uint32_t id = PR_ntohl(reinterpret_cast<uint32_t *>(setting)[0]) & 0xffffff;
     uint32_t value =  PR_ntohl(reinterpret_cast<uint32_t *>(setting)[1]);
 
     LOG3(("Settings ID %d, Flags %X, Value %d", id, flags, value));
 
     switch (id)
     {
-    case SETTINGS_TYPE_UPLOAD_BW:
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_UL_BW, value);
-      break;
-
-    case SETTINGS_TYPE_DOWNLOAD_BW:
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_DL_BW, value);
-      break;
-
-    case SETTINGS_TYPE_RTT:
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_RTT, value);
-      break;
-
     case SETTINGS_TYPE_MAX_CONCURRENT:
       self->mMaxConcurrent = value;
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_MAX_STREAMS, value);
       self->ProcessPending();
       break;
 
-    case SETTINGS_TYPE_CWND:
-      if (flags & PERSIST_VALUE)
-      {
-        nsRefPtr<nsHttpConnectionInfo> ci;
-        self->GetConnectionInfo(getter_AddRefs(ci));
-        if (ci)
-          gHttpHandler->ConnMgr()->ReportSpdyCWNDSetting(ci, value);
-      }
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_CWND, value);
-      break;
-
-    case SETTINGS_TYPE_DOWNLOAD_RETRANS_RATE:
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_RETRANS, value);
-      break;
-
     case SETTINGS_TYPE_INITIAL_WINDOW:
-      Telemetry::Accumulate(Telemetry::SPDY_SETTINGS_IW, value >> 10);
       {
         int32_t delta = value - self->mServerInitialStreamWindow;
         self->mServerInitialStreamWindow = value;
 
         // do not use SETTINGS to adjust the session window.
 
         // we need to add the delta to all open streams (delta can be negative)
         self->mStreamTransactionHash.Enumerate(UpdateServerRwinEnumerator,
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -742,77 +742,16 @@ nsHttpConnectionMgr::ReportSpdyConnectio
               "may have insufficient cert. Leave mapping in place but do not "
               "abandon this connection yet."));
     }
 
     ProcessPendingQ(ent->mConnInfo);
     PostEvent(&nsHttpConnectionMgr::OnMsgProcessAllSpdyPendingQ);
 }
 
-void
-nsHttpConnectionMgr::ReportSpdyCWNDSetting(nsHttpConnectionInfo *ci,
-                                           uint32_t cwndValue)
-{
-    MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
-
-    if (!gHttpHandler->UseSpdyPersistentSettings())
-        return;
-
-    if (!ci)
-        return;
-
-    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
-    if (!ent)
-        return;
-
-    ent = GetSpdyPreferredEnt(ent);
-    if (!ent) // just to be thorough - but that map should always exist
-        return;
-
-    cwndValue = std::max(2U, cwndValue);
-    cwndValue = std::min(128U, cwndValue);
-
-    ent->mSpdyCWND = cwndValue;
-    ent->mSpdyCWNDTimeStamp = TimeStamp::Now();
-    return;
-}
-
-// a value of 0 means no setting is available
-uint32_t
-nsHttpConnectionMgr::GetSpdyCWNDSetting(nsHttpConnectionInfo *ci)
-{
-    MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
-
-    if (!gHttpHandler->UseSpdyPersistentSettings())
-        return 0;
-
-    if (!ci)
-        return 0;
-
-    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
-    if (!ent)
-        return 0;
-
-    ent = GetSpdyPreferredEnt(ent);
-    if (!ent) // just to be thorough - but that map should always exist
-        return 0;
-
-    if (ent->mSpdyCWNDTimeStamp.IsNull())
-        return 0;
-
-    // For privacy tracking reasons, and the fact that CWND is not
-    // meaningful after some time, we don't honor stored CWND after 8
-    // hours.
-    TimeDuration age = TimeStamp::Now() - ent->mSpdyCWNDTimeStamp;
-    if (age.ToMilliseconds() > (1000 * 60 * 60 * 8))
-        return 0;
-
-    return ent->mSpdyCWND;
-}
-
 nsHttpConnectionMgr::nsConnectionEntry *
 nsHttpConnectionMgr::GetSpdyPreferredEnt(nsConnectionEntry *aOriginalEntry)
 {
     if (!gHttpHandler->IsSpdyEnabled() ||
         !gHttpHandler->CoalesceSpdy() ||
         aOriginalEntry->mCoalescingKeys.IsEmpty()) {
         return nullptr;
     }
@@ -3664,17 +3603,16 @@ nsHttpConnectionMgr::
 nsConnectionEntry::nsConnectionEntry(nsHttpConnectionInfo *ci)
     : mConnInfo(ci)
     , mPipelineState(PS_YELLOW)
     , mYellowGoodEvents(0)
     , mYellowBadEvents(0)
     , mYellowConnection(nullptr)
     , mGreenDepth(kPipelineOpen)
     , mPipeliningPenalty(0)
-    , mSpdyCWND(0)
     , mUsingSpdy(false)
     , mTestedSpdy(false)
     , mInPreferredHash(false)
     , mPreferIPv4(false)
     , mPreferIPv6(false)
 {
     MOZ_COUNT_CTOR(nsConnectionEntry);
     if (gHttpHandler->GetPipelineAggressive()) {
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -237,21 +237,16 @@ public:
     // that the network peer has closed the transport.
     nsresult CloseIdleConnection(nsHttpConnection *);
 
     // The connection manager needs to know when a normal HTTP connection has been
     // upgraded to SPDY because the dispatch and idle semantics are a little
     // bit different.
     void ReportSpdyConnection(nsHttpConnection *, bool usingSpdy);
 
-    // A spdy server can supply cwnd information for the session that is used
-    // in future sessions to speed up the opening portions of the connection.
-    void ReportSpdyCWNDSetting(nsHttpConnectionInfo *host, uint32_t cwndValue);
-    uint32_t GetSpdyCWNDSetting(nsHttpConnectionInfo *host);
-
     bool     SupportsPipelining(nsHttpConnectionInfo *);
 
     bool GetConnectionData(nsTArray<HttpRetParams> *);
 
     void ResetIPFamilyPreference(nsHttpConnectionInfo *);
 
     uint16_t MaxRequestDelay() { return mMaxRequestDelay; }
 
@@ -361,21 +356,16 @@ private:
         // to build the hash key for hosts in the same ip pool.
         //
         // When a set of hosts are coalesced together one of them is marked
         // mSpdyPreferred. The mapping is maintained in the connection mananger
         // mSpdyPreferred hash.
         //
         nsTArray<nsCString> mCoalescingKeys;
 
-        // The value of a recevied SPDY settings type 5 previously received
-        // for this connection entry and the time it was set.
-        uint32_t            mSpdyCWND;
-        TimeStamp  mSpdyCWNDTimeStamp;
-
         // To have the UsingSpdy flag means some host with the same connection
         // entry has done NPN=spdy/* at some point. It does not mean every
         // connection is currently using spdy.
         bool mUsingSpdy;
 
         // mTestedSpdy is set after NPN negotiation has occurred and we know
         // with confidence whether a host speaks spdy or not (which is reflected
         // in mUsingSpdy). Before mTestedSpdy is set, handshake parallelism is
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -1451,70 +1451,70 @@
     "expires_in_version": "never",
     "kind": "exponential",
     "high": "3000",
     "n_buckets": 50,
     "extended_statistics_ok": true,
     "description": "SPDY: KB read per connection"
   },
   "SPDY_SETTINGS_UL_BW": {
-    "expires_in_version": "never",
+    "expires_in_version": "42",
     "kind": "exponential",
     "high": "10000",
     "n_buckets": 100,
     "extended_statistics_ok": true,
     "description": "SPDY: Settings Upload Bandwidth"
   },
   "SPDY_SETTINGS_DL_BW": {
-    "expires_in_version": "never",
+    "expires_in_version": "42",
     "kind": "exponential",
     "high": "10000",
     "n_buckets": 100,
     "extended_statistics_ok": true,
     "description": "SPDY: Settings Download Bandwidth"
   },
   "SPDY_SETTINGS_RTT": {
-    "expires_in_version": "never",
+    "expires_in_version": "42",
     "kind": "exponential",
     "high": "1000",
     "n_buckets": 100,
     "extended_statistics_ok": true,
     "description": "SPDY: Settings RTT"
   },
   "SPDY_SETTINGS_MAX_STREAMS": {
     "expires_in_version": "never",
     "kind": "exponential",
     "high": "5000",
     "n_buckets": 100,
     "extended_statistics_ok": true,
-    "description": "SPDY: Settings Max Streams parameter"
+    "description": "H2: Settings Max Streams parameter"
   },
   "SPDY_SETTINGS_CWND": {
-    "expires_in_version": "never",
+    "expires_in_version": "42",
     "kind": "exponential",
     "high": "500",
     "n_buckets": 50,
     "extended_statistics_ok": true,
     "description": "SPDY: Settings CWND (packets)"
   },
   "SPDY_SETTINGS_RETRANS": {
-    "expires_in_version": "never",
+    "expires_in_version": "42",
     "kind": "exponential",
     "high": "100",
     "n_buckets": 50,
     "extended_statistics_ok": true,
     "description": "SPDY: Retransmission Rate"
   },
   "SPDY_SETTINGS_IW": {
     "expires_in_version": "never",
     "kind": "exponential",
     "high": "1000",
     "n_buckets": 50,
     "extended_statistics_ok": true,
-    "description": "SPDY: Settings IW (rounded to KB)"
+    "description": "H2: Settings Initial Window (rounded to KB)"
   },
   "DISK_CACHE_CORRUPT_DETAILS": {
     "expires_in_version": "40",
     "kind": "enumerated",
     "n_values": 50,
     "description": "Why the HTTP disk cache was corrupted at startup"
   },
   "DISK_CACHE_REDUCTION_TRIAL": {