From 92e52710085cf31d44280ee415c3b5e0d4b58db2 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Wed, 7 Sep 2016 10:18:17 -0700 Subject: [PATCH] Backed out 17 changesets (bug 1277504, bug 1218577, bug 1218576) for ASAN bc2 failures a=backout Backed out changeset 62009556e4ad (bug 1218576) Backed out changeset 57f9849f0f8f (bug 1218576) Backed out changeset 9595c56c9db2 (bug 1277504) Backed out changeset 9d15ae92f2fa (bug 1218576) Backed out changeset ea7282078b05 (bug 1218576) Backed out changeset 848f4ef30978 (bug 1218576) Backed out changeset 1b6666eb3b81 (bug 1218576) Backed out changeset ce2a2dabb042 (bug 1218576) Backed out changeset 3caacb5c213b (bug 1218576) Backed out changeset c68fc5ad5ecf (bug 1218576) Backed out changeset 1678482b2fad (bug 1218576) Backed out changeset df28918fe236 (bug 1218576) Backed out changeset eb5dbe28ab20 (bug 1218576) Backed out changeset baf105cbe0c8 (bug 1218576) Backed out changeset 7fdd6b6ab594 (bug 1218576) Backed out changeset a0a4829d0ca0 (bug 1218577) Backed out changeset fc16cda7781b (bug 1218576) --- dom/base/test/browser_use_counters.js | 43 +- dom/ipc/ContentParent.cpp | 16 - dom/ipc/ContentParent.h | 4 - dom/ipc/PContent.ipdl | 8 - toolkit/components/telemetry/Telemetry.cpp | 19 - toolkit/components/telemetry/Telemetry.h | 17 - toolkit/components/telemetry/TelemetryComms.h | 84 -- .../telemetry/TelemetryHistogram.cpp | 764 ++++++------------ .../components/telemetry/TelemetryHistogram.h | 7 - .../components/telemetry/TelemetrySession.jsm | 124 +-- toolkit/components/telemetry/moz.build | 1 - toolkit/components/telemetry/nsITelemetry.idl | 6 - .../tests/unit/test_ChildHistograms.js | 39 +- toolkit/content/aboutTelemetry.css | 12 - toolkit/content/aboutTelemetry.js | 61 +- toolkit/content/aboutTelemetry.xhtml | 6 - 16 files changed, 351 insertions(+), 860 deletions(-) delete mode 100644 toolkit/components/telemetry/TelemetryComms.h diff --git a/dom/base/test/browser_use_counters.js b/dom/base/test/browser_use_counters.js index f92b117a5760..3e81bda081e8 100644 --- a/dom/base/test/browser_use_counters.js +++ b/dom/base/test/browser_use_counters.js @@ -3,7 +3,6 @@ requestLongerTimeout(2); var {Promise: promise} = Cu.import("resource://gre/modules/Promise.jsm", {}); -Cu.import("resource://gre/modules/Services.jsm"); const gHttpTestRoot = "http://example.com/browser/dom/base/test/"; @@ -105,18 +104,22 @@ function waitForPageLoad(browser) { }); } -function grabHistogramsFromContent(use_counter_middlefix, page_before = null) { - let telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); - let suffix = Services.appinfo.browserTabsRemoteAutostart ? "#content" : ""; - let gather = () => [ - telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_PAGE" + suffix).snapshot().sum, - telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_DOCUMENT" + suffix).snapshot().sum, - telemetry.getHistogramById("CONTENT_DOCUMENTS_DESTROYED" + suffix).snapshot().sum, - telemetry.getHistogramById("TOP_LEVEL_CONTENT_DOCUMENTS_DESTROYED" + suffix).snapshot().sum, - ]; - return BrowserTestUtils.waitForCondition(() => { - return page_before != telemetry.getHistogramById("USE_COUNTER2_" + use_counter_middlefix + "_PAGE" + suffix).snapshot().sum; - }).then(gather, gather); +function grabHistogramsFromContent(browser, use_counter_middlefix) { + return ContentTask.spawn(browser, { middlefix: use_counter_middlefix }, function* (arg) { + let telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); + function snapshot_histogram(name) { + return telemetry.getHistogramById(name).snapshot(); + } + + let histogram_page_name = "USE_COUNTER2_" + arg.middlefix + "_PAGE"; + let histogram_document_name = "USE_COUNTER2_" + arg.middlefix + "_DOCUMENT"; + let histogram_page = snapshot_histogram(histogram_page_name); + let histogram_document = snapshot_histogram(histogram_document_name); + let histogram_docs = snapshot_histogram("CONTENT_DOCUMENTS_DESTROYED"); + let histogram_toplevel_docs = snapshot_histogram("TOP_LEVEL_CONTENT_DOCUMENTS_DESTROYED"); + return [histogram_page.sum, histogram_document.sum, + histogram_docs.sum, histogram_toplevel_docs.sum]; + }); } var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix, check_documents=true) { @@ -130,7 +133,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + "file_use_counter_outer.html"); yield waitForPageLoad(gBrowser.selectedBrowser); @@ -148,7 +151,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix event.target.removeEventListener("load", listener, true); // We flush the main document first, then the iframe's document to - // ensure any propagation that might happen from content->parent should + // ensure any propagation that might happen from child->parent should // have already happened when counters are reported to telemetry. wu.forceUseCounterFlush(content.document); wu.forceUseCounterFlush(iframe.contentDocument); @@ -171,7 +174,7 @@ var check_use_counter_iframe = Task.async(function* (file, use_counter_middlefix // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); is(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); @@ -194,7 +197,7 @@ var check_use_counter_img = Task.async(function* (file, use_counter_middlefix) { // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + "file_use_counter_outer.html"); yield waitForPageLoad(gBrowser.selectedBrowser); @@ -236,7 +239,7 @@ var check_use_counter_img = Task.async(function* (file, use_counter_middlefix) { // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); is(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); is(histogram_document_after, histogram_document_before + 1, @@ -260,7 +263,7 @@ var check_use_counter_direct = Task.async(function* (file, use_counter_middlefix // interested in. let [histogram_page_before, histogram_document_before, histogram_docs_before, histogram_toplevel_docs_before] = - yield grabHistogramsFromContent(use_counter_middlefix); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); gBrowser.selectedBrowser.loadURI(gHttpTestRoot + file); yield ContentTask.spawn(gBrowser.selectedBrowser, null, function*() { @@ -289,7 +292,7 @@ var check_use_counter_direct = Task.async(function* (file, use_counter_middlefix // Grab histograms again and compare. let [histogram_page_after, histogram_document_after, histogram_docs_after, histogram_toplevel_docs_after] = - yield grabHistogramsFromContent(use_counter_middlefix, histogram_page_before); + yield grabHistogramsFromContent(gBrowser.selectedBrowser, use_counter_middlefix); (xfail ? todo_is : is)(histogram_page_after, histogram_page_before + 1, "page counts for " + use_counter_middlefix + " after are correct"); (xfail ? todo_is : is)(histogram_document_after, histogram_document_before + 1, diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index d94edbf8aac3..98c26585e4f8 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5404,19 +5404,3 @@ ContentParent::SendGetFilesResponseAndForget(const nsID& aUUID, Unused << SendGetFilesResponse(aUUID, aResult); } } - -bool -ContentParent::RecvAccumulateChildHistogram( - InfallibleTArray&& aAccumulations) -{ - Telemetry::AccumulateChild(aAccumulations); - return true; -} - -bool -ContentParent::RecvAccumulateChildKeyedHistogram( - InfallibleTArray&& aAccumulations) -{ - Telemetry::AccumulateChildKeyed(aAccumulations); - return true; -} diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index dada52716df6..f32d53f31460 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1134,10 +1134,6 @@ private: virtual bool RecvDeleteGetFilesRequest(const nsID& aID) override; - virtual bool RecvAccumulateChildHistogram( - InfallibleTArray&& aAccumulations) override; - virtual bool RecvAccumulateChildKeyedHistogram( - InfallibleTArray&& aAccumulations) override; public: void SendGetFilesResponseAndForget(const nsID& aID, const GetFilesResponseResult& aResult); diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 03aa98a39f01..b47af2b642cf 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -100,8 +100,6 @@ using mozilla::DataStorageType from "ipc/DataStorageIPCUtils.h"; using mozilla::DocShellOriginAttributes from "mozilla/ipc/BackgroundUtils.h"; using struct mozilla::layers::TextureFactoryIdentifier from "mozilla/layers/CompositorTypes.h"; using struct mozilla::dom::FlyWebPublishOptions from "mozilla/dom/FlyWebPublishOptionsIPCSerializer.h"; -using mozilla::Telemetry::Accumulation from "mozilla/TelemetryComms.h"; -using mozilla::Telemetry::KeyedAccumulation from "mozilla/TelemetryComms.h"; union ChromeRegistryItem { @@ -1218,12 +1216,6 @@ parent: async UnstoreAndBroadcastBlobURLUnregistration(nsCString url); - /** - * Messages for communicating child Telemetry to the parent process - */ - async AccumulateChildHistogram(Accumulation[] accumulations); - async AccumulateChildKeyedHistogram(KeyedAccumulation[] accumulations); - both: async AsyncMessage(nsString aMessage, CpowEntry[] aCpows, Principal aPrincipal, ClonedMessageData aData); diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 1d9a3a0da3e2..f9854c70cd21 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -2359,13 +2359,6 @@ TelemetryImpl::ClearScalars() return NS_OK; } -NS_IMETHODIMP -TelemetryImpl::FlushBatchedChildTelemetry() -{ - TelemetryHistogram::IPCTimerFired(nullptr, nullptr); - return NS_OK; -} - size_t TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) { @@ -2811,18 +2804,6 @@ AccumulateTimeDelta(ID aHistogram, TimeStamp start, TimeStamp end) static_cast((end - start).ToMilliseconds())); } -void -AccumulateChild(const nsTArray& aAccumulations) -{ - TelemetryHistogram::AccumulateChild(aAccumulations); -} - -void -AccumulateChildKeyed(const nsTArray& aAccumulations) -{ - TelemetryHistogram::AccumulateChildKeyed(aAccumulations); -} - void ClearHistogram(ID aId) { diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index 0204036f4b03..6084c209898e 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -33,9 +33,6 @@ namespace HangMonitor { } // namespace HangMonitor namespace Telemetry { -struct Accumulation; -struct KeyedAccumulation; - enum TimerResolution { Millisecond, Microsecond @@ -128,20 +125,6 @@ void AccumulateCategorical(ID id, const nsCString& label); */ void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end = TimeStamp::Now()); -/** - * Accumulate child data into child histograms - * - * @param aAccumulations - accumulation actions to perform - */ -void AccumulateChild(const nsTArray& aAccumulations); - -/** - * Accumulate child data into child keyed histograms - * - * @param aAccumulations - accumulation actions to perform - */ -void AccumulateChildKeyed(const nsTArray& aAccumulations); - /** * This clears the data for a histogram in TelemetryHistogramEnums.h. * diff --git a/toolkit/components/telemetry/TelemetryComms.h b/toolkit/components/telemetry/TelemetryComms.h deleted file mode 100644 index 0f2d888e3198..000000000000 --- a/toolkit/components/telemetry/TelemetryComms.h +++ /dev/null @@ -1,84 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2; -*- */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - */ - -#ifndef Telemetry_Comms_h__ -#define Telemetry_Comms_h__ - -#include "ipc/IPCMessageUtils.h" - -namespace mozilla { -namespace Telemetry { - -enum ID : uint32_t; - -struct Accumulation -{ - mozilla::Telemetry::ID mId; - uint32_t mSample; -}; - -struct KeyedAccumulation -{ - mozilla::Telemetry::ID mId; - uint32_t mSample; - nsCString mKey; -}; - -} // namespace Telemetry -} // namespace mozilla - -namespace IPC { - -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::Accumulation paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - aMsg->WriteUInt32(aParam.mId); - WriteParam(aMsg, aParam.mSample); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!aMsg->ReadUInt32(aIter, reinterpret_cast(&(aResult->mId))) || - !ReadParam(aMsg, aIter, &(aResult->mSample))) { - return false; - } - - return true; - } -}; - -template<> -struct -ParamTraits -{ - typedef mozilla::Telemetry::KeyedAccumulation paramType; - - static void Write(Message* aMsg, const paramType& aParam) - { - aMsg->WriteUInt32(aParam.mId); - WriteParam(aMsg, aParam.mSample); - WriteParam(aMsg, aParam.mKey); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) - { - if (!aMsg->ReadUInt32(aIter, reinterpret_cast(&(aResult->mId))) || - !ReadParam(aMsg, aIter, &(aResult->mSample)) || - !ReadParam(aMsg, aIter, &(aResult->mKey))) { - return false; - } - - return true; - } -}; - -} // namespace IPC - -#endif // Telemetry_Comms_h__ diff --git a/toolkit/components/telemetry/TelemetryHistogram.cpp b/toolkit/components/telemetry/TelemetryHistogram.cpp index 969adfaa3d46..c8711bee1243 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.cpp +++ b/toolkit/components/telemetry/TelemetryHistogram.cpp @@ -14,13 +14,9 @@ #include "nsClassHashtable.h" #include "nsITelemetry.h" -#include "mozilla/dom/ContentChild.h" #include "mozilla/dom/ToJSValue.h" -#include "mozilla/Atomics.h" #include "mozilla/StartupTimeline.h" #include "mozilla/StaticMutex.h" -#include "mozilla/StaticPtr.h" -#include "mozilla/Unused.h" #include "TelemetryCommon.h" #include "TelemetryHistogram.h" @@ -35,9 +31,6 @@ using base::FlagHistogram; using base::LinearHistogram; using mozilla::StaticMutex; using mozilla::StaticMutexAutoLock; -using mozilla::StaticAutoPtr; -using mozilla::Telemetry::Accumulation; -using mozilla::Telemetry::KeyedAccumulation; //////////////////////////////////////////////////////////////////////// @@ -99,7 +92,6 @@ using mozilla::Telemetry::KeyedAccumulation; #define EXPIRED_ID "__expired__" #define SUBSESSION_HISTOGRAM_PREFIX "sub#" #define KEYED_HISTOGRAM_NAME_SEPARATOR "#" -#define CHILD_HISTOGRAM_SUFFIX "#content" namespace { @@ -193,13 +185,6 @@ AddonMapType gAddonMap; // The singleton StatisticsRecorder object for this process. base::StatisticsRecorder* gStatisticsRecorder = nullptr; -// For batching and sending child process accumulations to the parent -nsITimer* gIPCTimer = nullptr; -mozilla::Atomic gIPCTimerArmed(false); -mozilla::Atomic gIPCTimerArming(false); -StaticAutoPtr> gAccumulations; -StaticAutoPtr> gKeyedAccumulations; - } // namespace @@ -219,12 +204,6 @@ const mozilla::Telemetry::ID kRecordingInitiallyDisabledIDs[] = { mozilla::Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD }; -// Sending each remote accumulation immediately places undue strain on the -// IPC subsystem. Batch the remote accumulations for a period of time before -// sending them all at once. This value was chosen as a balance between data -// timeliness and performance (see bug 1218576) -const uint32_t kBatchTimeoutMs = 2000; - } // namespace @@ -338,16 +317,6 @@ HistogramInfo::label_id(const char* label, uint32_t* labelId) const return NS_ERROR_FAILURE; } -bool -StringEndsWith(const std::string& name, const std::string& suffix) -{ - if (name.size() < suffix.size()) { - return false; - } - - return name.compare(name.size() - suffix.size(), suffix.size(), suffix) == 0; -} - } // namespace @@ -432,18 +401,6 @@ internal_HistogramGet(const char *name, const char *expiration, return NS_OK; } -CharPtrEntryType* -internal_GetHistogramMapEntry(const char* name) -{ - nsDependentCString histogramName(name); - NS_NAMED_LITERAL_CSTRING(suffix, CHILD_HISTOGRAM_SUFFIX); - if (!StringEndsWith(histogramName, suffix)) { - return gHistogramMap.GetEntry(name); - } - auto root = Substring(histogramName, 0, histogramName.Length() - suffix.Length()); - return gHistogramMap.GetEntry(PromiseFlatCString(root).get()); -} - nsresult internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) { @@ -451,7 +408,7 @@ internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) return NS_ERROR_FAILURE; } - CharPtrEntryType *entry = internal_GetHistogramMapEntry(name); + CharPtrEntryType *entry = gHistogramMap.GetEntry(name); if (!entry) { return NS_ERROR_INVALID_ARG; } @@ -461,12 +418,10 @@ internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) // O(1) histogram lookup by numeric id nsresult -internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, - bool child = false) +internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) { static Histogram* knownHistograms[mozilla::Telemetry::HistogramCount] = {0}; - static Histogram* knownChildHistograms[mozilla::Telemetry::HistogramCount] = {0}; - Histogram *h = child ? knownChildHistograms[id] : knownHistograms[id]; + Histogram *h = knownHistograms[id]; if (h) { *ret = h; return NS_OK; @@ -477,15 +432,8 @@ internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, return NS_ERROR_FAILURE; } - nsCString histogramName; - histogramName.Append(p.id()); - if (child) { - histogramName.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - } - - nsresult rv = internal_HistogramGet(histogramName.get(), p.expiration(), - p.histogramType, p.min, p.max, - p.bucketCount, true, &h); + nsresult rv = internal_HistogramGet(p.id(), p.expiration(), p.histogramType, + p.min, p.max, p.bucketCount, true, &h); if (NS_FAILED(rv)) return rv; @@ -505,11 +453,7 @@ internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret, } #endif - if (child) { - *ret = knownChildHistograms[id] = h; - } else { - *ret = knownHistograms[id] = h; - } + *ret = knownHistograms[id] = h; return NS_OK; } @@ -523,9 +467,7 @@ internal_GetHistogramByName(const nsACString &name, Histogram **ret) return rv; } - bool isChild = StringEndsWith(name, - NS_LITERAL_CSTRING(CHILD_HISTOGRAM_SUFFIX)); - rv = internal_GetHistogramByEnumId(id, ret, isChild); + rv = internal_GetHistogramByEnumId(id, ret); if (NS_FAILED(rv)) return rv; @@ -581,7 +523,6 @@ internal_CloneHistogram(const nsACString& newName, } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) - Histogram* internal_GetSubsessionHistogram(Histogram& existing) { @@ -592,14 +533,9 @@ internal_GetSubsessionHistogram(Histogram& existing) return nullptr; } - bool isChild = StringEndsWith(existing.histogram_name(), - CHILD_HISTOGRAM_SUFFIX); - static Histogram* subsession[mozilla::Telemetry::HistogramCount] = {}; - static Histogram* subsessionChild[mozilla::Telemetry::HistogramCount] = {}; - Histogram* cached = isChild ? subsessionChild[id] : subsession[id]; - if (cached) { - return cached; + if (subsession[id]) { + return subsession[id]; } NS_NAMED_LITERAL_CSTRING(prefix, SUBSESSION_HISTOGRAM_PREFIX); @@ -609,15 +545,10 @@ internal_GetSubsessionHistogram(Histogram& existing) } nsCString subsessionName(prefix); - subsessionName.Append(existing.histogram_name().c_str()); + subsessionName.Append(existingName); - Histogram* clone = internal_CloneHistogram(subsessionName, id, existing); - if (isChild) { - subsessionChild[id] = clone; - } else { - subsession[id] = clone; - } - return clone; + subsession[id] = internal_CloneHistogram(subsessionName, id, existing); + return subsession[id]; } #endif @@ -666,13 +597,26 @@ internal_HistogramAdd(Histogram& histogram, int32_t value) return internal_HistogramAdd(histogram, value, dataset); } +nsresult +internal_HistogramAddCategorical(mozilla::Telemetry::ID id, const nsCString& label) +{ + uint32_t labelId = 0; + if (NS_FAILED(gHistograms[id].label_id(label.get(), &labelId))) { + return NS_ERROR_ILLEGAL_VALUE; + } + + Histogram* h = nullptr; + nsresult rv = internal_GetHistogramByEnumId(id, &h); + if (NS_FAILED(rv)) { + return rv; + } + + return internal_HistogramAdd(*h, labelId); +} + void internal_HistogramClear(Histogram& aHistogram, bool onlySubsession) { - MOZ_ASSERT(XRE_IsParentProcess()); - if (!XRE_IsParentProcess()) { - return; - } if (!onlySubsession) { aHistogram.Clear(); } @@ -868,8 +812,6 @@ public: nsresult Add(const nsCString& key, uint32_t aSample); void Clear(bool subsession); - nsresult GetEnumId(mozilla::Telemetry::ID& id); - private: typedef nsBaseHashtableET KeyedHistogramEntry; typedef AutoHashtable KeyedHistogramMapType; @@ -1012,10 +954,6 @@ KeyedHistogram::Add(const nsCString& key, uint32_t sample) void KeyedHistogram::Clear(bool onlySubsession) { - MOZ_ASSERT(XRE_IsParentProcess()); - if (!XRE_IsParentProcess()) { - return; - } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) for (auto iter = mSubsessionMap.Iter(); !iter.Done(); iter.Next()) { iter.Get()->mData->Clear(); @@ -1103,12 +1041,6 @@ KeyedHistogram::GetJSSnapshot(JSContext* cx, JS::Handle obj, return NS_OK; } -nsresult -KeyedHistogram::GetEnumId(mozilla::Telemetry::ID& id) -{ - return internal_GetHistogramEnumId(mName.get(), &id); -} - } // namespace @@ -1134,305 +1066,6 @@ internal_GetKeyedHistogramById(const nsACString &name) } // namespace -//////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////// -// -// PRIVATE: functions related to addon histograms - -namespace { - -// Compute the name to pass into Histogram for the addon histogram -// 'name' from the addon 'id'. We can't use 'name' directly because it -// might conflict with other histograms in other addons or even with our -// own. -void -internal_AddonHistogramName(const nsACString &id, const nsACString &name, - nsACString &ret) -{ - ret.Append(id); - ret.Append(':'); - ret.Append(name); -} - -bool -internal_CreateHistogramForAddon(const nsACString &name, - AddonHistogramInfo &info) -{ - Histogram *h; - nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), "never", - info.histogramType, info.min, info.max, - info.bucketCount, true, &h); - if (NS_FAILED(rv)) { - return false; - } - // Don't let this histogram be reported via the normal means - // (e.g. Telemetry.registeredHistograms); we'll make it available in - // other ways. - h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); - info.h = h; - return true; -} - -bool -internal_AddonHistogramReflector(AddonHistogramEntryType *entry, - JSContext *cx, JS::Handle obj) -{ - AddonHistogramInfo &info = entry->mData; - - // Never even accessed the histogram. - if (!info.h) { - // Have to force creation of HISTOGRAM_FLAG histograms. - if (info.histogramType != nsITelemetry::HISTOGRAM_FLAG) - return true; - - if (!internal_CreateHistogramForAddon(entry->GetKey(), info)) { - return false; - } - } - - if (internal_IsEmpty(info.h)) { - return true; - } - - JS::Rooted snapshot(cx, JS_NewPlainObject(cx)); - if (!snapshot) { - // Just consider this to be skippable. - return true; - } - switch (internal_ReflectHistogramSnapshot(cx, snapshot, info.h)) { - case REFLECT_FAILURE: - case REFLECT_CORRUPT: - return false; - case REFLECT_OK: - const nsACString &histogramName = entry->GetKey(); - if (!JS_DefineProperty(cx, obj, PromiseFlatCString(histogramName).get(), - snapshot, JSPROP_ENUMERATE)) { - return false; - } - break; - } - return true; -} - -bool -internal_AddonReflector(AddonEntryType *entry, JSContext *cx, - JS::Handle obj) -{ - const nsACString &addonId = entry->GetKey(); - JS::Rooted subobj(cx, JS_NewPlainObject(cx)); - if (!subobj) { - return false; - } - - AddonHistogramMapType *map = entry->mData; - if (!(map->ReflectIntoJS(internal_AddonHistogramReflector, cx, subobj) - && JS_DefineProperty(cx, obj, PromiseFlatCString(addonId).get(), - subobj, JSPROP_ENUMERATE))) { - return false; - } - return true; -} - -} // namespace - - -//////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////// -// -// PRIVATE: thread-unsafe helpers for the external interface - -// This is a StaticMutex rather than a plain Mutex (1) so that -// it gets initialised in a thread-safe manner the first time -// it is used, and (2) because it is never de-initialised, and -// a normal Mutex would show up as a leak in BloatView. StaticMutex -// also has the "OffTheBooks" property, so it won't show as a leak -// in BloatView. -static StaticMutex gTelemetryHistogramMutex; - -namespace { - -void -internal_SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) -{ - if (!internal_IsHistogramEnumId(aID)) { - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); - return; - } - - if (gHistograms[aID].keyed) { - const nsDependentCString id(gHistograms[aID].id()); - KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); - if (keyed) { - keyed->SetRecordingEnabled(aEnabled); - return; - } - } else { - Histogram *h; - nsresult rv = internal_GetHistogramByEnumId(aID, &h); - if (NS_SUCCEEDED(rv)) { - h->SetRecordingEnabled(aEnabled); - return; - } - } - - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); -} - -void internal_armIPCTimerMainThread() -{ - MOZ_ASSERT(NS_IsMainThread()); - gIPCTimerArming = false; - if (gIPCTimerArmed) { - return; - } - if (!gIPCTimer) { - CallCreateInstance(NS_TIMER_CONTRACTID, &gIPCTimer); - } - if (gIPCTimer) { - gIPCTimer->InitWithFuncCallback(TelemetryHistogram::IPCTimerFired, - nullptr, kBatchTimeoutMs, - nsITimer::TYPE_ONE_SHOT); - gIPCTimerArmed = true; - } -} - -void internal_armIPCTimer() -{ - if (gIPCTimerArmed || gIPCTimerArming) { - return; - } - gIPCTimerArming = true; - if (NS_IsMainThread()) { - internal_armIPCTimerMainThread(); - } else { - NS_DispatchToMainThread(NS_NewRunnableFunction([]() -> void { - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - internal_armIPCTimerMainThread(); - })); - } -} - -bool -internal_RemoteAccumulate(mozilla::Telemetry::ID aId, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - return false; - } - if (!gAccumulations) { - gAccumulations = new nsTArray(); - } - gAccumulations->AppendElement(Accumulation{aId, aSample}); - internal_armIPCTimer(); - return true; -} - -bool -internal_RemoteAccumulate(mozilla::Telemetry::ID aId, - const nsCString& aKey, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - return false; - } - if (!gKeyedAccumulations) { - gKeyedAccumulations = new nsTArray(); - } - gKeyedAccumulations->AppendElement(KeyedAccumulation{aId, aSample, aKey}); - internal_armIPCTimer(); - return true; -} - -void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) -{ - if (!internal_CanRecordBase() || - internal_RemoteAccumulate(aHistogram, aSample)) { - return; - } - Histogram *h; - nsresult rv = internal_GetHistogramByEnumId(aHistogram, &h); - if (NS_SUCCEEDED(rv)) { - internal_HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); - } -} - -void -internal_Accumulate(mozilla::Telemetry::ID aID, - const nsCString& aKey, uint32_t aSample) -{ - if (!gInitDone || !internal_CanRecordBase() || - internal_RemoteAccumulate(aID, aKey, aSample)) { - return; - } - const HistogramInfo& th = gHistograms[aID]; - KeyedHistogram* keyed - = internal_GetKeyedHistogramById(nsDependentCString(th.id())); - MOZ_ASSERT(keyed); - keyed->Add(aKey, aSample); -} - -void -internal_Accumulate(Histogram& aHistogram, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - internal_HistogramAdd(aHistogram, aSample); - return; - } - - mozilla::Telemetry::ID id; - nsresult rv = internal_GetHistogramEnumId(aHistogram.histogram_name().c_str(), &id); - if (NS_SUCCEEDED(rv)) { - internal_RemoteAccumulate(id, aSample); - } -} - -void -internal_Accumulate(KeyedHistogram& aKeyed, - const nsCString& aKey, uint32_t aSample) -{ - if (XRE_IsParentProcess()) { - aKeyed.Add(aKey, aSample); - return; - } - - mozilla::Telemetry::ID id; - if (NS_SUCCEEDED(aKeyed.GetEnumId(id))) { - internal_RemoteAccumulate(id, aKey, aSample); - } -} - -void -internal_AccumulateChild(mozilla::Telemetry::ID aId, uint32_t aSample) -{ - if (!internal_CanRecordBase()) { - return; - } - Histogram* h; - nsresult rv = internal_GetHistogramByEnumId(aId, &h, true); - if (NS_SUCCEEDED(rv)) { - internal_HistogramAdd(*h, aSample, gHistograms[aId].dataset); - } else { - NS_WARNING("NS_FAILED GetHistogramByEnumId for CHILD"); - } -} - -void -internal_AccumulateChildKeyed(mozilla::Telemetry::ID aId, - const nsCString& aKey, uint32_t aSample) -{ - if (!gInitDone || !internal_CanRecordBase()) { - return; - } - const HistogramInfo& th = gHistograms[aId]; - nsCString id; - id.Append(th.id()); - id.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); - MOZ_ASSERT(keyed); - keyed->Add(aKey, aSample); -} - -} // namespace - - //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // @@ -1475,47 +1108,52 @@ internal_JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) return true; } - uint32_t value = 0; - mozilla::Telemetry::ID id; + // If we don't have an argument for the count histogram, assume an increment of 1. + // Otherwise, make sure to run some sanity checks on the argument. if ((type == base::CountHistogram::COUNT_HISTOGRAM) && (args.length() == 0)) { - // If we don't have an argument for the count histogram, assume an increment of 1. - // Otherwise, make sure to run some sanity checks on the argument. - value = 1; - } else if (type == base::LinearHistogram::LINEAR_HISTOGRAM && + internal_HistogramAdd(*h, 1); + return true; + } + + // For categorical histograms we allow passing a string argument that specifies the label. + mozilla::Telemetry::ID id; + if (type == base::LinearHistogram::LINEAR_HISTOGRAM && (args.length() > 0) && args[0].isString() && NS_SUCCEEDED(internal_GetHistogramEnumId(h->histogram_name().c_str(), &id)) && gHistograms[id].histogramType == nsITelemetry::HISTOGRAM_CATEGORICAL) { - // For categorical histograms we allow passing a string argument that specifies the label. nsAutoJSString label; if (!label.init(cx, args[0])) { JS_ReportError(cx, "Invalid string parameter"); return false; } - nsresult rv = gHistograms[id].label_id(NS_ConvertUTF16toUTF8(label).get(), &value); + nsresult rv = internal_HistogramAddCategorical(id, NS_ConvertUTF16toUTF8(label)); if (NS_FAILED(rv)) { JS_ReportError(cx, "Unknown label for categorical histogram"); return false; } - } else { - // All other accumulations expect one numerical argument. - if (!args.length()) { - JS_ReportError(cx, "Expected one argument"); - return false; - } - if (!(args[0].isNumber() || args[0].isBoolean())) { - JS_ReportError(cx, "Not a number"); - return false; - } - - if (!JS::ToUint32(cx, args[0], &value)) { - JS_ReportError(cx, "Failed to convert argument"); - return false; - } + return true; } - internal_Accumulate(*h, value); + // All other accumulations expect one numerical argument. + int32_t value = 0; + if (!args.length()) { + JS_ReportError(cx, "Expected one argument"); + return false; + } + + if (!(args[0].isNumber() || args[0].isBoolean())) { + JS_ReportError(cx, "Not a number"); + return false; + } + + if (!JS::ToInt32(cx, args[0], &value)) { + JS_ReportError(cx, "Failed to convert argument"); + return false; + } + + internal_HistogramAdd(*h, value); return true; } @@ -1763,7 +1401,7 @@ internal_JSKeyedHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) } } - internal_Accumulate(*keyed, NS_ConvertUTF16toUTF8(key), value); + keyed->Add(NS_ConvertUTF16toUTF8(key), value); return true; } @@ -1913,11 +1551,184 @@ internal_WrapAndReturnKeyedHistogram(KeyedHistogram *h, JSContext *cx, } // namespace +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// PRIVATE: functions related to addon histograms + +namespace { + +// Compute the name to pass into Histogram for the addon histogram +// 'name' from the addon 'id'. We can't use 'name' directly because it +// might conflict with other histograms in other addons or even with our +// own. +void +internal_AddonHistogramName(const nsACString &id, const nsACString &name, + nsACString &ret) +{ + ret.Append(id); + ret.Append(':'); + ret.Append(name); +} + +bool +internal_CreateHistogramForAddon(const nsACString &name, + AddonHistogramInfo &info) +{ + Histogram *h; + nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), "never", + info.histogramType, info.min, info.max, + info.bucketCount, true, &h); + if (NS_FAILED(rv)) { + return false; + } + // Don't let this histogram be reported via the normal means + // (e.g. Telemetry.registeredHistograms); we'll make it available in + // other ways. + h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); + info.h = h; + return true; +} + +bool +internal_AddonHistogramReflector(AddonHistogramEntryType *entry, + JSContext *cx, JS::Handle obj) +{ + AddonHistogramInfo &info = entry->mData; + + // Never even accessed the histogram. + if (!info.h) { + // Have to force creation of HISTOGRAM_FLAG histograms. + if (info.histogramType != nsITelemetry::HISTOGRAM_FLAG) + return true; + + if (!internal_CreateHistogramForAddon(entry->GetKey(), info)) { + return false; + } + } + + if (internal_IsEmpty(info.h)) { + return true; + } + + JS::Rooted snapshot(cx, JS_NewPlainObject(cx)); + if (!snapshot) { + // Just consider this to be skippable. + return true; + } + switch (internal_ReflectHistogramSnapshot(cx, snapshot, info.h)) { + case REFLECT_FAILURE: + case REFLECT_CORRUPT: + return false; + case REFLECT_OK: + const nsACString &histogramName = entry->GetKey(); + if (!JS_DefineProperty(cx, obj, PromiseFlatCString(histogramName).get(), + snapshot, JSPROP_ENUMERATE)) { + return false; + } + break; + } + return true; +} + +bool +internal_AddonReflector(AddonEntryType *entry, JSContext *cx, + JS::Handle obj) +{ + const nsACString &addonId = entry->GetKey(); + JS::Rooted subobj(cx, JS_NewPlainObject(cx)); + if (!subobj) { + return false; + } + + AddonHistogramMapType *map = entry->mData; + if (!(map->ReflectIntoJS(internal_AddonHistogramReflector, cx, subobj) + && JS_DefineProperty(cx, obj, PromiseFlatCString(addonId).get(), + subobj, JSPROP_ENUMERATE))) { + return false; + } + return true; +} + +} // namespace + + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// PRIVATE: thread-unsafe helpers for the external interface + +namespace { + +void +internal_SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) +{ + if (!internal_IsHistogramEnumId(aID)) { + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); + return; + } + + if (gHistograms[aID].keyed) { + const nsDependentCString id(gHistograms[aID].id()); + KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); + if (keyed) { + keyed->SetRecordingEnabled(aEnabled); + return; + } + } else { + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aID, &h); + if (NS_SUCCEEDED(rv)) { + h->SetRecordingEnabled(aEnabled); + return; + } + } + + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); +} + +void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) +{ + if (!internal_CanRecordBase()) { + return; + } + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aHistogram, &h); + if (NS_SUCCEEDED(rv)) { + internal_HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); + } +} + +void +internal_Accumulate(mozilla::Telemetry::ID aID, + const nsCString& aKey, uint32_t aSample) +{ + if (!gInitDone || !internal_CanRecordBase()) { + return; + } + const HistogramInfo& th = gHistograms[aID]; + KeyedHistogram* keyed + = internal_GetKeyedHistogramById(nsDependentCString(th.id())); + MOZ_ASSERT(keyed); + keyed->Add(aKey, aSample); +} + +} // namespace + + //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // // EXTERNALLY VISIBLE FUNCTIONS in namespace TelemetryHistogram:: +// This is a StaticMutex rather than a plain Mutex (1) so that +// it gets initialised in a thread-safe manner the first time +// it is used, and (2) because it is never de-initialised, and +// a normal Mutex would show up as a leak in BloatView. StaticMutex +// also has the "OffTheBooks" property, so it won't show as a leak +// in BloatView. +static StaticMutex gTelemetryHistogramMutex; + // All of these functions are actually in namespace TelemetryHistogram::, // but the ::TelemetryHistogram prefix is given explicitly. This is // because it is critical to see which calls from these functions are @@ -1981,16 +1792,6 @@ void TelemetryHistogram::InitializeGlobalState(bool canRecordBase, const nsDependentCString expiration(h.expiration()); gKeyedHistograms.Put(id, new KeyedHistogram(id, expiration, h.histogramType, h.min, h.max, h.bucketCount, h.dataset)); - if (XRE_IsParentProcess()) { - // We must create registered child keyed histograms as well or else the - // same code in TelemetrySession.jsm that fails without parent keyed - // histograms will fail without child keyed histograms. - nsCString childId(id); - childId.AppendLiteral(CHILD_HISTOGRAM_SUFFIX); - gKeyedHistograms.Put(childId, - new KeyedHistogram(id, expiration, h.histogramType, - h.min, h.max, h.bucketCount, h.dataset)); - } } // Some Telemetry histograms depend on the value of C++ constants and hardcode @@ -2021,11 +1822,6 @@ void TelemetryHistogram::DeInitializeGlobalState() gHistogramMap.Clear(); gKeyedHistograms.Clear(); gAddonMap.Clear(); - gAccumulations = nullptr; - gKeyedAccumulations = nullptr; - if (gIPCTimer) { - NS_RELEASE(gIPCTimer); - } gInitDone = false; } @@ -2131,7 +1927,12 @@ TelemetryHistogram::Accumulate(const char* name, uint32_t sample) if (NS_FAILED(rv)) { return; } - internal_Accumulate(id, sample); + + Histogram *h; + rv = internal_GetHistogramByEnumId(id, &h); + if (NS_SUCCEEDED(rv)) { + internal_HistogramAdd(*h, sample, gHistograms[id].dataset); + } } void @@ -2154,52 +1955,7 @@ TelemetryHistogram::AccumulateCategorical(mozilla::Telemetry::ID aId, const nsCString& label) { StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - uint32_t labelId = 0; - if (NS_FAILED(gHistograms[aId].label_id(label.get(), &labelId))) { - return; - } - internal_Accumulate(aId, labelId); -} - -void -TelemetryHistogram::AccumulateChild(const nsTArray& aAccumulations) -{ - MOZ_ASSERT(XRE_IsParentProcess()); - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - for (uint32_t i = 0; i < aAccumulations.Length(); ++i) { - bool isValid = internal_IsHistogramEnumId(aAccumulations[i].mId); - MOZ_ASSERT(isValid); - if (!isValid) { - continue; - } - internal_AccumulateChild(aAccumulations[i].mId, aAccumulations[i].mSample); - } -} - -void -TelemetryHistogram::AccumulateChildKeyed(const nsTArray& aAccumulations) -{ - MOZ_ASSERT(XRE_IsParentProcess()); - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (!internal_CanRecordBase()) { - return; - } - for (uint32_t i = 0; i < aAccumulations.Length(); ++i) { - bool isValid = internal_IsHistogramEnumId(aAccumulations[i].mId); - MOZ_ASSERT(isValid); - if (!isValid) { - continue; - } - internal_AccumulateChildKeyed(aAccumulations[i].mId, - aAccumulations[i].mKey, - aAccumulations[i].mSample); - } + internal_HistogramAddCategorical(aId, label); } void @@ -2344,8 +2100,6 @@ TelemetryHistogram::CreateHistogramSnapshots(JSContext *cx, mozilla::DebugOnly rv = internal_GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h); MOZ_ASSERT(NS_SUCCEEDED(rv)); - rv = internal_GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h, true); - MOZ_ASSERT(NS_SUCCEEDED(rv)); } } @@ -2609,43 +2363,3 @@ TelemetryHistogram::GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf } return n; } - -// This method takes the lock only to double-buffer the batched telemetry. -// It releases the lock before calling out to IPC code which can (and does) -// Accumulate (which would deadlock) -// -// To ensure we don't loop IPCTimerFired->AccumulateChild->arm timer, we don't -// unset gIPCTimerArmed until the IPC completes -// -// This function must be called on the main thread, otherwise IPC will fail. -void -TelemetryHistogram::IPCTimerFired(nsITimer* aTimer, void* aClosure) -{ - MOZ_ASSERT(NS_IsMainThread()); - nsTArray accumulationsToSend; - nsTArray keyedAccumulationsToSend; - { - StaticMutexAutoLock locker(gTelemetryHistogramMutex); - if (gAccumulations) { - accumulationsToSend.SwapElements(*gAccumulations); - } - if (gKeyedAccumulations) { - keyedAccumulationsToSend.SwapElements(*gKeyedAccumulations); - } - } - - mozilla::dom::ContentChild* contentChild = mozilla::dom::ContentChild::GetSingleton(); - mozilla::Unused << NS_WARN_IF(!contentChild); - if (contentChild) { - if (accumulationsToSend.Length()) { - mozilla::Unused << - NS_WARN_IF(!contentChild->SendAccumulateChildHistogram(accumulationsToSend)); - } - if (keyedAccumulationsToSend.Length()) { - mozilla::Unused << - NS_WARN_IF(!contentChild->SendAccumulateChildKeyedHistogram(keyedAccumulationsToSend)); - } - } - - gIPCTimerArmed = false; -} diff --git a/toolkit/components/telemetry/TelemetryHistogram.h b/toolkit/components/telemetry/TelemetryHistogram.h index ff19a5efae1d..4bb83dbf5f30 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.h +++ b/toolkit/components/telemetry/TelemetryHistogram.h @@ -8,8 +8,6 @@ #include "mozilla/TelemetryHistogramEnums.h" -#include "mozilla/TelemetryComms.h" - // This module is internal to Telemetry. It encapsulates Telemetry's // histogram accumulation and storage logic. It should only be used by // Telemetry.cpp. These functions should not be used anywhere else. @@ -44,9 +42,6 @@ void Accumulate(const char* name, const nsCString& key, uint32_t sample); void AccumulateCategorical(mozilla::Telemetry::ID aId, const nsCString& aLabel); -void AccumulateChild(const nsTArray& aAccumulations); -void AccumulateChildKeyed(const nsTArray& aAccumulations); - void ClearHistogram(mozilla::Telemetry::ID aId); @@ -107,8 +102,6 @@ GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf); size_t GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf aMallocSizeOf); -void -IPCTimerFired(nsITimer* aTimer, void* aClosure); } // namespace TelemetryHistogram #endif // TelemetryHistogram_h__ diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index c20fdbe22d3a..68d7f0e8c981 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -41,11 +41,6 @@ const REASON_TEST_PING = "test-ping"; const REASON_ENVIRONMENT_CHANGE = "environment-change"; const REASON_SHUTDOWN = "shutdown"; -const HISTOGRAM_SUFFIXES = { - PARENT: "", - CONTENT: "#content", -} - const ENVIRONMENT_CHANGE_LISTENER = "TelemetrySession::onEnvironmentChange"; const MS_IN_ONE_HOUR = 60 * 60 * 1000; @@ -62,6 +57,7 @@ const PREF_UNIFIED = PREF_BRANCH + "unified"; const MESSAGE_TELEMETRY_PAYLOAD = "Telemetry:Payload"; +const MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD = "Telemetry:GetChildPayload"; const MESSAGE_TELEMETRY_THREAD_HANGS = "Telemetry:ChildThreadHangs"; const MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS = "Telemetry:GetChildThreadHangs"; const MESSAGE_TELEMETRY_USS = "Telemetry:USS"; @@ -544,6 +540,13 @@ this.TelemetrySession = Object.freeze({ getPayload: function(reason, clearSubsession = false) { return Impl.getPayload(reason, clearSubsession); }, + /** + * Asks the content processes to send their payloads. + * @returns Object + */ + requestChildPayloads: function() { + return Impl.requestChildPayloads(); + }, /** * Returns a promise that resolves to an array of thread hang stats from content processes, one entry per process. * The structure of each entry is identical to that of "threadHangStats" in nsITelemetry. @@ -881,28 +884,23 @@ var Impl = { }, getHistograms: function getHistograms(subsession, clearSubsession) { - this._log.trace("getHistograms - subsession: " + subsession + - ", clearSubsession: " + clearSubsession); + this._log.trace("getHistograms - subsession: " + subsession + ", clearSubsession: " + clearSubsession); let registered = Telemetry.registeredHistograms(this.getDatasetType(), []); - if (this._testing == false) { - // Omit telemetry test histograms outside of tests. - registered = registered.filter(n => !n.startsWith("TELEMETRY_TEST_")); - } - registered = registered.concat(registered.map(n => "STARTUP_" + n)); - let hls = subsession ? Telemetry.snapshotSubsessionHistograms(clearSubsession) : Telemetry.histogramSnapshots; let ret = {}; for (let name of registered) { - for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) { - if (name + suffix in hls) { - if (!(suffix in ret)) { - ret[suffix] = {}; + for (let n of [name, "STARTUP_" + name]) { + if (n in hls) { + // Omit telemetry test histograms outside of tests. + if (n.startsWith('TELEMETRY_TEST_') && this._testing == false) { + this._log.trace("getHistograms - Skipping test histogram: " + n); + } else { + ret[n] = this.packHistogram(hls[n]); } - ret[suffix][name] = this.packHistogram(hls[name + suffix]); } } } @@ -930,41 +928,36 @@ var Impl = { }, getKeyedHistograms: function(subsession, clearSubsession) { - this._log.trace("getKeyedHistograms - subsession: " + subsession + - ", clearSubsession: " + clearSubsession); + this._log.trace("getKeyedHistograms - subsession: " + subsession + ", clearSubsession: " + clearSubsession); let registered = Telemetry.registeredKeyedHistograms(this.getDatasetType(), []); - if (this._testing == false) { - // Omit telemetry test histograms outside of tests. - registered = registered.filter(id => !id.startsWith("TELEMETRY_TEST_")); - } let ret = {}; for (let id of registered) { - for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) { - let keyed = Telemetry.getKeyedHistogramById(id + suffix); - let snapshot = null; - if (subsession) { - snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear() - : keyed.subsessionSnapshot(); - } else { - snapshot = keyed.snapshot(); - } + // Omit telemetry test histograms outside of tests. + if (id.startsWith('TELEMETRY_TEST_') && this._testing == false) { + this._log.trace("getKeyedHistograms - Skipping test histogram: " + id); + continue; + } + let keyed = Telemetry.getKeyedHistogramById(id); + let snapshot = null; + if (subsession) { + snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear() + : keyed.subsessionSnapshot(); + } else { + snapshot = keyed.snapshot(); + } - let keys = Object.keys(snapshot); - if (keys.length == 0) { - // Skip empty keyed histogram. - continue; - } + let keys = Object.keys(snapshot); + if (keys.length == 0) { + // Skip empty keyed histogram. + continue; + } - if (!(suffix in ret)) { - ret[suffix] = {}; - } - ret[suffix][id] = {}; - for (let key of keys) { - ret[suffix][id][key] = this.packHistogram(snapshot[key]); - } + ret[id] = {}; + for (let key of keys) { + ret[id][key] = this.packHistogram(snapshot[key]); } } @@ -1250,12 +1243,12 @@ var Impl = { // This allows wrapping data retrieval calls in a try-catch block so that // failures don't break the rest of the ping assembly. - const protect = (fn, defaultReturn = null) => { + const protect = (fn) => { try { return fn(); } catch (ex) { this._log.error("assemblePayloadWithMeasurements - caught exception", ex); - return defaultReturn; + return null; } }; @@ -1263,6 +1256,8 @@ var Impl = { let payloadObj = { ver: PAYLOAD_VERSION, simpleMeasurements: simpleMeasurements, + histograms: protect(() => this.getHistograms(isSubsession, clearSubsession)), + keyedHistograms: protect(() => this.getKeyedHistograms(isSubsession, clearSubsession)), }; // Add extended set measurements common to chrome & content processes @@ -1277,21 +1272,14 @@ var Impl = { return payloadObj; } - // Additional payload for chrome process. - let histograms = protect(() => this.getHistograms(isSubsession, clearSubsession), {}); - let keyedHistograms = protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {}); - payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {}; - payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {}; + // Set the scalars for the parent process. payloadObj.processes = { parent: { scalars: protect(() => this.getScalars(isSubsession, clearSubsession)), - }, - content: { - histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT], - keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT], - }, + } }; + // Additional payload for chrome process. payloadObj.info = info; // Add extended set measurements for chrome process. @@ -1533,6 +1521,7 @@ var Impl = { } Services.obs.addObserver(this, "content-child-shutdown", false); + cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD, this); cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_THREAD_HANGS, this); cpml.addMessageListener(MESSAGE_TELEMETRY_GET_CHILD_USS, this); @@ -1570,6 +1559,14 @@ var Impl = { let source = message.data.childUUID; delete message.data.childUUID; + for (let child of this._childTelemetry) { + if (child.source === source) { + // Update existing telemetry data. + child.payload = message.data; + return; + } + } + // Did not find existing child in this._childTelemetry. this._childTelemetry.push({ source: source, payload: message.data, @@ -1582,6 +1579,12 @@ var Impl = { break; } + case MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD: + { + // In child process, send the requested Telemetry payload + this.sendContentProcessPing("saved-session"); + break; + } case MESSAGE_TELEMETRY_THREAD_HANGS: { // Accumulate child thread hang stats from this child @@ -1761,6 +1764,11 @@ var Impl = { return this.getSessionPayload(reason, clearSubsession); }, + requestChildPayloads: function() { + this._log.trace("requestChildPayloads"); + ppmm.broadcastAsyncMessage(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD, {}); + }, + getChildThreadHangs: function getChildThreadHangs() { return new Promise((resolve) => { // Return immediately if there are no child processes to get stats from @@ -1838,7 +1846,7 @@ var Impl = { // content-child-shutdown is only registered for content processes. Services.obs.removeObserver(this, "content-child-shutdown"); this.uninstall(); - Telemetry.flushBatchedChildTelemetry(); + this.sendContentProcessPing(REASON_SAVED_SESSION); break; case TOPIC_CYCLE_COLLECTOR_BEGIN: diff --git a/toolkit/components/telemetry/moz.build b/toolkit/components/telemetry/moz.build index 7e38babc2d44..b2cff123823a 100644 --- a/toolkit/components/telemetry/moz.build +++ b/toolkit/components/telemetry/moz.build @@ -19,7 +19,6 @@ EXPORTS.mozilla += [ '!TelemetryScalarEnums.h', 'ProcessedStack.h', 'Telemetry.h', - 'TelemetryComms.h', 'ThreadHangStats.h', ] diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index a07072a3213b..c992b2a8c32d 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -416,10 +416,4 @@ interface nsITelemetry : nsISupports * Resets all the stored scalars. This is intended to be only used in tests. */ void clearScalars(); - - /** - * Immediately sends any Telemetry batched on this process to the parent - * process. This is intended only to be used on process shutdown. - */ - void flushBatchedChildTelemetry(); }; diff --git a/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js b/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js index ad30140d55e0..4233328b600f 100644 --- a/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js +++ b/toolkit/components/telemetry/tests/unit/test_ChildHistograms.js @@ -2,7 +2,6 @@ Cu.import("resource://gre/modules/TelemetryController.jsm", this); Cu.import("resource://gre/modules/TelemetrySession.jsm", this); Cu.import("resource://gre/modules/PromiseUtils.jsm", this); -Cu.import("resource://testing-common/ContentTaskUtils.jsm", this); const MESSAGE_TELEMETRY_PAYLOAD = "Telemetry:Payload"; const MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD = "Telemetry:GetChildPayload"; @@ -20,9 +19,6 @@ function run_child_test() { let countHist = Telemetry.getHistogramById("TELEMETRY_TEST_COUNT"); countHist.add(); countHist.add(); - let categHist = Telemetry.getHistogramById("TELEMETRY_TEST_CATEGORICAL"); - categHist.add("Label2"); - categHist.add("Label3"); let flagKeyed = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_FLAG"); flagKeyed.add("a", 1); @@ -31,19 +27,20 @@ function run_child_test() { countKeyed.add("a"); countKeyed.add("b"); countKeyed.add("b"); + + // Check payload values. + const payload = TelemetrySession.getPayload("test-ping"); + check_histogram_values(payload); } function check_histogram_values(payload) { const hs = payload.histograms; Assert.ok("TELEMETRY_TEST_COUNT" in hs, "Should have count test histogram."); Assert.ok("TELEMETRY_TEST_FLAG" in hs, "Should have flag test histogram."); - Assert.ok("TELEMETRY_TEST_CATEGORICAL" in hs, "Should have categorical test histogram."); Assert.equal(hs["TELEMETRY_TEST_COUNT"].sum, 2, "Count test histogram should have the right value."); Assert.equal(hs["TELEMETRY_TEST_FLAG"].sum, 1, "Flag test histogram should have the right value."); - Assert.equal(hs["TELEMETRY_TEST_CATEGORICAL"].sum, 3, - "Categorical test histogram should have the right sum."); const kh = payload.keyedHistograms; Assert.ok("TELEMETRY_TEST_KEYED_COUNT" in kh, "Should have keyed count test histogram."); @@ -64,6 +61,8 @@ add_task(function*() { run_child_test(); dump("... done with child test\n"); do_send_remote_message(MESSAGE_CHILD_TEST_DONE); + dump("... waiting for child payload collection\n"); + yield do_await_remote_message(MESSAGE_TELEMETRY_GET_CHILD_PAYLOAD); return; } @@ -81,20 +80,20 @@ add_task(function*() { let childPromise = run_test_in_child("test_ChildHistograms.js"); yield do_await_remote_message(MESSAGE_CHILD_TEST_DONE); - yield ContentTaskUtils.waitForCondition(() => { - let payload = TelemetrySession.getPayload("test-ping"); - return payload && - "processes" in payload && - "content" in payload.processes && - "histograms" in payload.processes.content && - "TELEMETRY_TEST_COUNT" in payload.processes.content.histograms; - }); + // Gather payload from child. + dump("... requesting child payloads\n"); + let promiseMessage = do_await_remote_message(MESSAGE_TELEMETRY_PAYLOAD); + TelemetrySession.requestChildPayloads(); + yield promiseMessage; + dump("... received child payload\n"); + + // Check child payload. const payload = TelemetrySession.getPayload("test-ping"); - Assert.ok("processes" in payload, "Should have processes section"); - Assert.ok("content" in payload.processes, "Should have child process section"); - Assert.ok("histograms" in payload.processes.content, "Child process section should have histograms."); - Assert.ok("keyedHistograms" in payload.processes.content, "Child process section should have keyed histograms."); - check_histogram_values(payload.processes.content); + Assert.ok("childPayloads" in payload, "Should have child payloads."); + Assert.equal(payload.childPayloads.length, 1, "Should have received one child payload so far."); + Assert.ok("histograms" in payload.childPayloads[0], "Child payload should have histograms."); + Assert.ok("keyedHistograms" in payload.childPayloads[0], "Child payload should have keyed histograms."); + check_histogram_values(payload.childPayloads[0]); do_test_finished(); }); diff --git a/toolkit/content/aboutTelemetry.css b/toolkit/content/aboutTelemetry.css index 6acf82c201fb..39ade6486080 100644 --- a/toolkit/content/aboutTelemetry.css +++ b/toolkit/content/aboutTelemetry.css @@ -229,14 +229,6 @@ body[dir="rtl"] .copy-node { display: inline; } -.processes-ui { - display: none; -} - -.has-data.expanded .processes-ui { - display: initial; -} - .filter-blocked { display: none; } @@ -265,7 +257,3 @@ body[dir="rtl"] .copy-node { font-size: larger; margin: 5px 0; } - -.process-picker { - margin: 0 0.5em; -} diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js index dd636990d48c..13aa26e382a3 100644 --- a/toolkit/content/aboutTelemetry.js +++ b/toolkit/content/aboutTelemetry.js @@ -306,10 +306,6 @@ var PingPicker = { .addEventListener("click", () => this._movePingIndex(1), false); document.getElementById("choose-payload") .addEventListener("change", () => displayPingData(gPingData), false); - document.getElementById("histograms-processes") - .addEventListener("change", () => displayPingData(gPingData), false); - document.getElementById("keyed-histograms-processes") - .addEventListener("change", () => displayPingData(gPingData), false); }, onPingSourceChanged: function() { @@ -1786,33 +1782,6 @@ function sortStartupMilestones(aSimpleMeasurements) { return result; } -function renderProcessList(ping, selectEl) { - removeAllChildNodes(selectEl); - let option = document.createElement("option"); - option.appendChild(document.createTextNode("parent")); - option.setAttribute("value", ""); - option.selected = true; - selectEl.appendChild(option); - - if (!("processes" in ping.payload)) { - selectEl.disabled = true; - return; - } - selectEl.disabled = false; - - for (let process of Object.keys(ping.payload.processes)) { - // TODO: parent hgrams are on root payload, not in payload.processes.parent - // When/If that gets moved, you'll need to remove this: - if (process === "parent") { - continue; - } - option = document.createElement("option"); - option.appendChild(document.createTextNode(process)); - option.setAttribute("value", process); - selectEl.appendChild(option); - } -} - function renderPayloadList(ping) { // Rebuild the payload select with options: // Parent Payload (selected) @@ -1881,11 +1850,9 @@ function displayPingData(ping, updatePayloadList = false) { const keysHeader = bundle.GetStringFromName("keysHeader"); const valuesHeader = bundle.GetStringFromName("valuesHeader"); - // Update the payload list and process lists + // Update the payload list if (updatePayloadList) { renderPayloadList(ping); - renderProcessList(ping, document.getElementById("histograms-processes")); - renderProcessList(ping, document.getElementById("keyed-histograms-processes")); } // Show general data. @@ -1962,18 +1929,8 @@ function displayPingData(ping, updatePayloadList = false) { removeAllChildNodes(hgramDiv); let histograms = payload.histograms; - - let hgramsSelect = document.getElementById("histograms-processes"); - let hgramsOption = hgramsSelect.selectedOptions.item(0); - let hgramsProcess = hgramsOption.getAttribute("value"); - if (hgramsProcess && - "processes" in ping.payload && - hgramsProcess in ping.payload.processes) { - histograms = ping.payload.processes[hgramsProcess].histograms; - } - hasData = Object.keys(histograms).length > 0; - setHasData("histograms-section", hasData || hgramsSelect.options.length); + setHasData("histograms-section", hasData); if (hasData) { for (let [name, hgram] of Object.entries(histograms)) { @@ -1993,18 +1950,8 @@ function displayPingData(ping, updatePayloadList = false) { let keyedDiv = document.getElementById("keyed-histograms"); removeAllChildNodes(keyedDiv); + setHasData("keyed-histograms-section", false); let keyedHistograms = payload.keyedHistograms; - - let keyedHgramsSelect = document.getElementById("keyed-histograms-processes"); - let keyedHgramsOption = keyedHgramsSelect.selectedOptions.item(0); - let keyedHgramsProcess = keyedHgramsOption.getAttribute("value"); - if (keyedHgramsProcess && - "processes" in ping.payload && - keyedHgramsProcess in ping.payload.processes) { - keyedHistograms = ping.payload.processes[keyedHgramsProcess].keyedHistograms; - } - - setHasData("keyed-histograms-section", keyedHgramsSelect.options.length); if (keyedHistograms) { let hasData = false; for (let [id, keyed] of Object.entries(keyedHistograms)) { @@ -2013,7 +1960,7 @@ function displayPingData(ping, updatePayloadList = false) { KeyedHistogram.render(keyedDiv, id, keyed, {unpacked: true}); } } - setHasData("keyed-histograms-section", hasData || keyedHgramsSelect.options.length); + setHasData("keyed-histograms-section", hasData); } // Show addon histogram data diff --git a/toolkit/content/aboutTelemetry.xhtml b/toolkit/content/aboutTelemetry.xhtml index 8049e3057812..b6d1a80300ad 100644 --- a/toolkit/content/aboutTelemetry.xhtml +++ b/toolkit/content/aboutTelemetry.xhtml @@ -152,9 +152,6 @@ &aboutTelemetry.filterText; -
- -
@@ -164,9 +161,6 @@

&aboutTelemetry.keyedHistogramsSection;

&aboutTelemetry.toggle; &aboutTelemetry.emptySection; -
- -