Comment on attachment 8764588 [details] [diff] [review] patch 1 - expired keys in nsBrowserApp Review of attachment 8764588 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/Histograms.json @@ -774,5 @@ > "n_buckets": 21, > "description": "Number of low-commit-space events fired since last ping", > "cpp_guard": "XP_WIN" > }, > - "EARLY_GLUESTARTUP_READ_OPS": { Also remove them from histogram-whitelists.json.

Comment on attachment 8764589 [details] [diff] [review] patch 2 - network/cache Review of attachment 8764589 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/Histograms.json @@ -1657,5 @@ > "expires_in_version": "never", > "kind": "boolean", > "description": "Fraction of sockets that used a nsConnectionEntry with history - size 300." > }, > - "DISK_CACHE_CORRUPT_DETAILS": { Also remove them from histogram-whitelists.json.

Comment on attachment 8764619 [details] [diff] [review] patch 6 - Search and other stuff Review of attachment 8764619 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/Histograms.json @@ -5093,5 @@ > "kind": "enumerated", > "n_values": 10, > "description": "Track click count on about:newtab tiles per index (0-8). For non-default row or column configurations all clicks into the '9' bucket. *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" > }, > - "BROWSERPROVIDER_XUL_IMPORT_TIME": { Also remove them from histogram-whitelists.json. @@ -6732,5 @@ > - "kind": "linear", > - "low": 5, > - "high": 145, > - "n_buckets": 30, > - "description": "The number of distinct pairs (first-party site, third-party site attempting to set cookie) for which the third-party cookie has been accepted. Sites are considered identical if they have the same eTLD + 1. Measures are normalized per 24h. *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" Lets make this bug block bug 1156565 then.

Comment on attachment 8764591 [details] [diff] [review] patch 3 - network/cache (2) Review of attachment 8764591 [details] [diff] [review]: ----------------------------------------------------------------- Remove the probes from histogram_whitelist. Also, please seek an r? from a cache service dev, as I'm not sure I understand the syntax around those locks :S

Comment on attachment 8764603 [details] [diff] [review] patch 4 - random things Review of attachment 8764603 [details] [diff] [review]: ----------------------------------------------------------------- Remove probes from histograms_whitelist

Comment on attachment 8764615 [details] [diff] [review] patch 5 - random things (2) Review of attachment 8764615 [details] [diff] [review]: ----------------------------------------------------------------- And from the whitelists. ::: dom/base/nsGlobalWindow.cpp @@ -12463,1 @@ > gTimeoutsRecentlySet = 0; gTimeoutsRecentlySet is never read except for telemetry accumulation. I guess it should be removed. ::: layout/base/nsPresShell.cpp @@ +9792,5 @@ > if (mDocument->GetRootElement()) { > TimeDuration elapsed = TimeStamp::Now() - timerStart; > int32_t intElapsed = int32_t(elapsed.ToMilliseconds()); > > + if (mDocument->GetRootElement()->IsXULElement() && mIsActive) { shouldn't this be if(!mDocument->GetRootElement()->IsXULElement() ?

Comment on attachment 8764849 [details] [diff] [review] patch 4 - random things Review of attachment 8764849 [details] [diff] [review]: ----------------------------------------------------------------- LGTM

Comment on attachment 8764855 [details] [diff] [review] patch 5 - random things (2) Review of attachment 8764855 [details] [diff] [review]: ----------------------------------------------------------------- LGTM

Comment on attachment 8764848 [details] [diff] [review] patch 3 - network/cache (2) Honza, does this look right to you? Feel free to run by Michal as well as needed.

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e1001d232f8a Remove some non-used telemetry IDs - part 1 - expired keys in nsBrowserApp, r=gfritzsche https://hg.mozilla.org/integration/mozilla-inbound/rev/4bf710c1a503 Remove some non-used telemetry IDs - part 2 - network/cache, r=gfritzsche https://hg.mozilla.org/integration/mozilla-inbound/rev/3af9c0fa2ba6 Remove some non-used telemetry IDs - part 3 - random things, r=chutten https://hg.mozilla.org/integration/mozilla-inbound/rev/27b1dd843116 Remove some non-used telemetry IDs - part 4 - random things (2), r=chutten https://hg.mozilla.org/integration/mozilla-inbound/rev/81120708f321 Remove some non-used telemetry IDs - part 5 - Search and other stuff, r=gfritzsche

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d3892088d89b Remove some non-used telemetry IDs - part 6 - histogram-whitelists.json fixed CLOSED TREE, r=me

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=30783958&repo=mozilla-inbound

Comment on attachment 8764849 [details] [diff] [review] patch 4 - random things Review of attachment 8764849 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/histogram-whitelists.json @@ -555,5 @@ > "HTTP_TRANSACTION_USE_ALTSVC_OE", > - "IDLE_NOTIFY_BACK_LISTENERS", > - "IDLE_NOTIFY_BACK_MS", > - "IDLE_NOTIFY_IDLE_LISTENERS", > - "IDLE_NOTIFY_IDLE_MS", Missed this the first time 'round. You didn't remove this hgram, but removed it from the whitelist. This is causing the bustage.

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3531fd033083 Backed out changeset d3892088d89b https://hg.mozilla.org/integration/mozilla-inbound/rev/28dbd9c39d3e Backed out changeset 81120708f321 https://hg.mozilla.org/integration/mozilla-inbound/rev/1704aab64d91 Backed out changeset 27b1dd843116 https://hg.mozilla.org/integration/mozilla-inbound/rev/5297b24d7720 Backed out changeset 3af9c0fa2ba6 https://hg.mozilla.org/integration/mozilla-inbound/rev/03b169a1cd5c Backed out changeset 4bf710c1a503 https://hg.mozilla.org/integration/mozilla-inbound/rev/c51fe95aa69a Backed out changeset e1001d232f8a for bustage on a CLOSED TREE

Comment on attachment 8764848 [details] [diff] [review] patch 3 - network/cache (2) Review of attachment 8764848 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/nsIdleService.cpp @@ -542,5 @@ > return NS_OK; > } > > // Mark all idle services as non-idle, and calculate the next idle timeout. > - Telemetry::AutoTimer<Telemetry::IDLE_NOTIFY_BACK_MS> timer; I don't see these removed from the Histograms.json (and other) files

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b022cc28da9b Remove some non-used telemetry IDs - part 1 - expired keys in nsBrowserApp, r=gfritzsche https://hg.mozilla.org/integration/mozilla-inbound/rev/33d663a3d351 Remove some non-used telemetry IDs - part 2 - network/cache, r=gfritzsche https://hg.mozilla.org/integration/mozilla-inbound/rev/dff93c4f631d Remove some non-used telemetry IDs - part 3 - random things, r=chutten https://hg.mozilla.org/integration/mozilla-inbound/rev/0ac748f4d677 Remove some non-used telemetry IDs - part 4 - random things (2), r=chutten https://hg.mozilla.org/integration/mozilla-inbound/rev/3896ec4f3a39 Remove some non-used telemetry IDs - part 5 - Search and other stuff, r=gfritzsche

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f3c807b23268 Remove some non-used telemetry IDs - part 6 - wrong IDs removed - CLOSED TREE, r=me

> Missed this the first time 'round. You didn't remove this hgram, but removed > it from the whitelist. This is causing the bustage. Yeah. I realized it by myself. what happened was that half of the patch 3 went into patch 4. I dream an automatic 'hg qref'. I pushed a fix and a new version of patch 3.

Comment on attachment 8766184 [details] [diff] [review] patch 3 - network/cache (2) Review of attachment 8766184 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/cache/nsDiskCacheMap.cpp @@ -1415,5 @@ > // would help. > } > > - // We want this after the lock to prove that flushing a file isn't that expensive > - Telemetry::AutoTimer<Telemetry::NETWORK_DISK_CACHE_REVALIDATION> totalTimer; I don't see this removed from histograms.json ::: netwerk/cache/nsDiskCacheStreams.cpp @@ -334,5 @@ > - mozilla::Telemetry::ID id; > - if (NS_IsMainThread()) > - id = mozilla::Telemetry::NETWORK_DISK_CACHE_STREAMIO_CLOSE_MAIN_THREAD; > - else > - id = mozilla::Telemetry::NETWORK_DISK_CACHE_STREAMIO_CLOSE; I don't see these removed from histograms.json ::: widget/nsIdleService.cpp @@ -542,5 @@ > return NS_OK; > } > > // Mark all idle services as non-idle, and calculate the next idle timeout. > - Telemetry::AutoTimer<Telemetry::IDLE_NOTIFY_BACK_MS> timer; if this is a "network/cache" patch, then these probes don't belong here. you remove them from histogram-whitelists.json in this patch but not from Histograms.json. move to it's own patch.

Comment on attachment 8766236 [details] [diff] [review] patch 3 - network/cache (2) Review of attachment 8766236 [details] [diff] [review]: ----------------------------------------------------------------- thanks! now it's correct.

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7ed188787902 Remove some non-used telemetry IDs - part 7 - network/cache (2), r=mayhemer

https://hg.mozilla.org/mozilla-central/rev/b022cc28da9b https://hg.mozilla.org/mozilla-central/rev/33d663a3d351 https://hg.mozilla.org/mozilla-central/rev/dff93c4f631d https://hg.mozilla.org/mozilla-central/rev/0ac748f4d677 https://hg.mozilla.org/mozilla-central/rev/3896ec4f3a39 https://hg.mozilla.org/mozilla-central/rev/f3c807b23268

(In reply to Chris H-C :chutten from comment #23) > Comment on attachment 8764849 [details] [diff] [review] > patch 4 - random things > > Review of attachment 8764849 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/components/telemetry/histogram-whitelists.json > @@ -555,5 @@ > > "HTTP_TRANSACTION_USE_ALTSVC_OE", > > - "IDLE_NOTIFY_BACK_LISTENERS", > > - "IDLE_NOTIFY_BACK_MS", > > - "IDLE_NOTIFY_IDLE_LISTENERS", > > - "IDLE_NOTIFY_IDLE_MS", > > Missed this the first time 'round. You didn't remove this hgram, but removed > it from the whitelist. This is causing the bustage. Andrea, did you clear up this r-? It looks like that patch landed?

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/86e45db70ad2 part 7 - Remove some non-used telemetry IDs - widget directory, r=gfritzsche

(In reply to Georg Fritzsche [:gfritzsche] [away Jun 24 - Jul 3] from comment #37) > (In reply to Chris H-C :chutten from comment #23) > > Comment on attachment 8764849 [details] [diff] [review] > > patch 4 - random things > > > > Review of attachment 8764849 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: toolkit/components/telemetry/histogram-whitelists.json > > @@ -555,5 @@ > > > "HTTP_TRANSACTION_USE_ALTSVC_OE", > > > - "IDLE_NOTIFY_BACK_LISTENERS", > > > - "IDLE_NOTIFY_BACK_MS", > > > - "IDLE_NOTIFY_IDLE_LISTENERS", > > > - "IDLE_NOTIFY_IDLE_MS", > > > > Missed this the first time 'round. You didn't remove this hgram, but removed > > it from the whitelist. This is causing the bustage. > > Andrea, did you clear up this r-? It looks like that patch landed?

> > Andrea, did you clear up this r-? It looks like that patch landed? At the beginning it was a r+. The I make a mess with the ordering of the patches and it was backed out. In the meantime I fixed the issue and I landed it again. This r- was in between the back-out and the rest. So yes, the patch is landed. If you want to take a look the landed patch is: https://hg.mozilla.org/integration/mozilla-inbound/rev/0ac748f4d677