Bug 1366294 - Part 1 - Remove base::StatisticsRecorder. r=chutten
The Chromium IPC histogram code used the StatisticsRecorder object for storage.
This is keyed by histogram name, which doesn't match our storage reality anymore.
Instead we use a name to refer to a set of histogram instances that record data from different processes, as well as separating session and subsession data.
Consequently we need to rewrite this storage, which means StatisticsRecorder is not used anymore.
MozReview-Commit-ID: 1LC7YubpKaD
--- a/ipc/chromium/src/base/histogram.cc
+++ b/ipc/chromium/src/base/histogram.cc
@@ -995,182 +995,9 @@ void CustomHistogram::InitializedCustomB
SetBucketRange(index, custom_ranges[index]);
ResetRangeChecksum();
}
double CustomHistogram::GetBucketSize(Count current, size_t i) const {
return 1;
}
-//------------------------------------------------------------------------------
-// The next section handles global (central) support for all histograms, as well
-// as startup/teardown of this service.
-//------------------------------------------------------------------------------
-
-// This singleton instance should be started during the single threaded portion
-// of main(), and hence it is not thread safe. It initializes globals to
-// provide support for all future calls.
-StatisticsRecorder::StatisticsRecorder() {
- DCHECK(!histograms_);
- if (lock_ == NULL) {
- // This will leak on purpose. It's the only way to make sure we won't race
- // against the static uninitialization of the module while one of our
- // static methods relying on the lock get called at an inappropriate time
- // during the termination phase. Since it's a static data member, we will
- // leak one per process, which would be similar to the instance allocated
- // during static initialization and released only on process termination.
- lock_ = new base::Lock;
- }
- base::AutoLock auto_lock(*lock_);
- histograms_ = new HistogramMap;
-}
-
-StatisticsRecorder::~StatisticsRecorder() {
- DCHECK(histograms_ && lock_);
-
- if (dump_on_exit_) {
- std::string output;
- WriteGraph("", &output);
- CHROMIUM_LOG(INFO) << output;
- }
- // Clean up.
- HistogramMap* histograms = NULL;
- {
- base::AutoLock auto_lock(*lock_);
- histograms = histograms_;
- histograms_ = NULL;
- for (HistogramMap::iterator it = histograms->begin();
- histograms->end() != it;
- ++it) {
- // No other clients permanently hold Histogram references, so we
- // have the only one and it is safe to delete it.
- delete it->second;
- }
- }
- delete histograms;
- // We don't delete lock_ on purpose to avoid having to properly protect
- // against it going away after we checked for NULL in the static methods.
-}
-
-// static
-bool StatisticsRecorder::IsActive() {
- if (lock_ == NULL)
- return false;
- base::AutoLock auto_lock(*lock_);
- return NULL != histograms_;
-}
-
-Histogram* StatisticsRecorder::RegisterOrDeleteDuplicate(Histogram* histogram) {
- DCHECK(histogram->HasValidRangeChecksum());
- if (lock_ == NULL)
- return histogram;
- base::AutoLock auto_lock(*lock_);
- if (!histograms_)
- return histogram;
- const std::string name = histogram->histogram_name();
- HistogramMap::iterator it = histograms_->find(name);
- // Avoid overwriting a previous registration.
- if (histograms_->end() == it) {
- (*histograms_)[name] = histogram;
- } else {
- delete histogram; // We already have one by this name.
- histogram = it->second;
- }
- return histogram;
-}
-
-// static
-void StatisticsRecorder::WriteHTMLGraph(const std::string& query,
- std::string* output) {
- if (!IsActive())
- return;
- output->append("<html><head><title>About Histograms");
- if (!query.empty())
- output->append(" - " + query);
- output->append("</title>"
- // We'd like the following no-cache... but it doesn't work.
- // "<META HTTP-EQUIV=\"Pragma\" CONTENT=\"no-cache\">"
- "</head><body>");
-
- Histograms snapshot;
- GetSnapshot(query, &snapshot);
- for (Histograms::iterator it = snapshot.begin();
- it != snapshot.end();
- ++it) {
- (*it)->WriteHTMLGraph(output);
- output->append("<br><hr><br>");
- }
- output->append("</body></html>");
-}
-
-// static
-void StatisticsRecorder::WriteGraph(const std::string& query,
- std::string* output) {
- if (!IsActive())
- return;
- if (query.length())
- StringAppendF(output, "Collections of histograms for %s\n", query.c_str());
- else
- output->append("Collections of all histograms\n");
-
- Histograms snapshot;
- GetSnapshot(query, &snapshot);
- for (Histograms::iterator it = snapshot.begin();
- it != snapshot.end();
- ++it) {
- (*it)->WriteAscii(true, "\n", output);
- output->append("\n");
- }
-}
-
-// static
-void StatisticsRecorder::GetHistograms(Histograms* output) {
- if (lock_ == NULL)
- return;
- base::AutoLock auto_lock(*lock_);
- if (!histograms_)
- return;
- for (HistogramMap::iterator it = histograms_->begin();
- histograms_->end() != it;
- ++it) {
- DCHECK_EQ(it->first, it->second->histogram_name());
- output->push_back(it->second);
- }
-}
-
-bool StatisticsRecorder::FindHistogram(const std::string& name,
- Histogram** histogram) {
- if (lock_ == NULL)
- return false;
- base::AutoLock auto_lock(*lock_);
- if (!histograms_)
- return false;
- HistogramMap::iterator it = histograms_->find(name);
- if (histograms_->end() == it)
- return false;
- *histogram = it->second;
- return true;
-}
-
-// private static
-void StatisticsRecorder::GetSnapshot(const std::string& query,
- Histograms* snapshot) {
- if (lock_ == NULL)
- return;
- base::AutoLock auto_lock(*lock_);
- if (!histograms_)
- return;
- for (HistogramMap::iterator it = histograms_->begin();
- histograms_->end() != it;
- ++it) {
- if (it->first.find(query) != std::string::npos)
- snapshot->push_back(it->second);
- }
-}
-
-// static
-StatisticsRecorder::HistogramMap* StatisticsRecorder::histograms_ = NULL;
-// static
-base::Lock* StatisticsRecorder::lock_ = NULL;
-// static
-bool StatisticsRecorder::dump_on_exit_ = false;
-
} // namespace base
--- a/ipc/chromium/src/base/histogram.h
+++ b/ipc/chromium/src/base/histogram.h
@@ -499,18 +499,16 @@ class Histogram {
virtual uint32_t CalculateRangeChecksum() const;
// Finally, provide the state that changes with the addition of each new
// sample.
SampleSet sample_;
private:
- friend class StatisticsRecorder; // To allow it to delete duplicates.
-
// Post constructor initialization.
void Initialize();
// Checksum function for accumulating range values into a checksum.
static uint32_t Crc32(uint32_t sum, Sample range);
//----------------------------------------------------------------------------
// Helpers for emitting Ascii graphic. Each method appends data to output.
@@ -708,73 +706,11 @@ class CustomHistogram : public Histogram
// Initialize ranges_ mapping.
void InitializedCustomBucketRange(const std::vector<Sample>& custom_ranges);
virtual double GetBucketSize(Count current, size_t i) const;
DISALLOW_COPY_AND_ASSIGN(CustomHistogram);
};
-//------------------------------------------------------------------------------
-// StatisticsRecorder handles all histograms in the system. It provides a
-// general place for histograms to register, and supports a global API for
-// accessing (i.e., dumping, or graphing) the data in all the histograms.
-
-class StatisticsRecorder {
- public:
- typedef std::vector<Histogram*> Histograms;
-
- StatisticsRecorder();
-
- ~StatisticsRecorder();
-
- // Find out if histograms can now be registered into our list.
- static bool IsActive();
-
- // Register, or add a new histogram to the collection of statistics. If an
- // identically named histogram is already registered, then the argument
- // |histogram| will deleted. The returned value is always the registered
- // histogram (either the argument, or the pre-existing registered histogram).
- static Histogram* RegisterOrDeleteDuplicate(Histogram* histogram);
-
- // Methods for printing histograms. Only histograms which have query as
- // a substring are written to output (an empty string will process all
- // registered histograms).
- static void WriteHTMLGraph(const std::string& query, std::string* output);
- static void WriteGraph(const std::string& query, std::string* output);
-
- // Method for extracting histograms which were marked for use by UMA.
- static void GetHistograms(Histograms* output);
-
- // Find a histogram by name. It matches the exact name. This method is thread
- // safe. If a matching histogram is not found, then the |histogram| is
- // not changed.
- static bool FindHistogram(const std::string& query, Histogram** histogram);
-
- static bool dump_on_exit() { return dump_on_exit_; }
-
- static void set_dump_on_exit(bool enable) { dump_on_exit_ = enable; }
-
- // GetSnapshot copies some of the pointers to registered histograms into the
- // caller supplied vector (Histograms). Only histograms with names matching
- // query are returned. The query must be a substring of histogram name for its
- // pointer to be copied.
- static void GetSnapshot(const std::string& query, Histograms* snapshot);
-
-
- private:
- // We keep all registered histograms in a map, from name to histogram.
- typedef std::map<std::string, Histogram*> HistogramMap;
-
- static HistogramMap* histograms_;
-
- // lock protects access to the above map.
- static Lock* lock_;
-
- // Dump all known histograms to log.
- static bool dump_on_exit_;
-
- DISALLOW_COPY_AND_ASSIGN(StatisticsRecorder);
-};
-
} // namespace base
#endif // BASE_METRICS_HISTOGRAM_H_
--- a/js/xpconnect/src/XPCShellImpl.cpp
+++ b/js/xpconnect/src/XPCShellImpl.cpp
@@ -30,18 +30,16 @@
#include "BackstagePass.h"
#include "nsIScriptSecurityManager.h"
#include "nsIPrincipal.h"
#include "nsJSUtils.h"
#include "gfxPrefs.h"
#include "nsIXULRuntime.h"
#include "GeckoProfiler.h"
-#include "base/histogram.h"
-
#ifdef ANDROID
#include <android/log.h>
#endif
#ifdef XP_WIN
#include "mozilla/widget/AudioSession.h"
#include <windows.h>
#if defined(MOZ_SANDBOX)
@@ -1220,21 +1218,16 @@ XRE_XPCShellMain(int argc, char** argv,
gErrFile = stderr;
gOutFile = stdout;
gInFile = stdin;
NS_LogInit();
mozilla::LogModule::Init();
- // A initializer to initialize histogram collection
- // used by telemetry.
- auto telStats =
- mozilla::MakeUnique<base::StatisticsRecorder>();
-
char aLocal;
profiler_init(&aLocal);
if (PR_GetEnv("MOZ_CHAOSMODE")) {
ChaosFeature feature = ChaosFeature::Any;
long featureInt = strtol(PR_GetEnv("MOZ_CHAOSMODE"), nullptr, 16);
if (featureInt) {
// NOTE: MOZ_CHAOSMODE=0 or a non-hex value maps to Any feature.
@@ -1554,18 +1547,16 @@ XRE_XPCShellMain(int argc, char** argv,
if (!XRE_ShutdownTestShell())
NS_ERROR("problem shutting down testshell");
// no nsCOMPtrs are allowed to be alive when you call NS_ShutdownXPCOM
rv = NS_ShutdownXPCOM( nullptr );
MOZ_ASSERT(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");
- telStats = nullptr;
-
#ifdef MOZ_CRASHREPORTER
// Shut down the crashreporter service to prevent leaking some strings it holds.
if (CrashReporter::GetEnabled())
CrashReporter::UnsetExceptionHandler();
#endif
// This must precede NS_LogTerm(), otherwise xpcshell return non-zero
// during some tests, which causes failures.
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -2411,26 +2411,16 @@ SetProfileDir(nsIFile* aProfD)
nsAutoString profDirPath;
nsresult rv = aProfD->GetPath(profDirPath);
if (NS_FAILED(rv)) {
return;
}
sTelemetryIOObserver->AddPath(profDirPath, NS_LITERAL_STRING("{profile}"));
}
-void CreateStatisticsRecorder()
-{
- TelemetryHistogram::CreateStatisticsRecorder();
-}
-
-void DestroyStatisticsRecorder()
-{
- TelemetryHistogram::DestroyStatisticsRecorder();
-}
-
// Scalar API C++ Endpoints
void
ScalarAdd(mozilla::Telemetry::ScalarID aId, uint32_t aVal)
{
TelemetryScalar::Add(aId, aVal);
}
--- a/toolkit/components/telemetry/Telemetry.h
+++ b/toolkit/components/telemetry/Telemetry.h
@@ -41,23 +41,16 @@ struct KeyedScalarAction;
struct ChildEventData;
enum TimerResolution {
Millisecond,
Microsecond
};
/**
- * Create and destroy the underlying base::StatisticsRecorder singleton.
- * Creation has to be done very early in the startup sequence.
- */
-void CreateStatisticsRecorder();
-void DestroyStatisticsRecorder();
-
-/**
* Initialize the Telemetry service on the main thread at startup.
*/
void Init();
/**
* Adds sample to a histogram defined in TelemetryHistogramEnums.h
*
* @param id - histogram id
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -23,17 +23,16 @@
#include "TelemetryCommon.h"
#include "TelemetryHistogram.h"
#include "ipc/TelemetryIPCAccumulator.h"
#include "base/histogram.h"
using base::Histogram;
-using base::StatisticsRecorder;
using base::BooleanHistogram;
using base::CountHistogram;
using base::FlagHistogram;
using base::LinearHistogram;
using mozilla::StaticMutex;
using mozilla::StaticMutexAutoLock;
using mozilla::Telemetry::Accumulation;
using mozilla::Telemetry::KeyedAccumulation;
@@ -143,18 +142,16 @@ struct HistogramInfo {
};
enum reflectStatus {
REFLECT_OK,
REFLECT_CORRUPT,
REFLECT_FAILURE
};
-typedef StatisticsRecorder::Histograms::iterator HistogramIterator;
-
} // namespace
////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////
//
// PRIVATE STATE, SHARED BY ALL THREADS
@@ -170,19 +167,16 @@ HistogramMapType gHistogramMap(mozilla::
KeyedHistogramMapType gKeyedHistograms;
bool gCorruptHistograms[mozilla::Telemetry::HistogramCount];
// This is for gHistograms, gHistogramStringTable
#include "TelemetryHistogramData.inc"
-// The singleton StatisticsRecorder object for this process.
-base::StatisticsRecorder* gStatisticsRecorder = nullptr;
-
} // namespace
////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////
//
// PRIVATE CONSTANTS
@@ -1805,34 +1799,16 @@ internal_WrapAndReturnKeyedHistogram(Key
// 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
// to another function in this interface. Mis-identifying "inwards
// calls" from "calls to another function in this interface" will lead
// to deadlocking and/or races. See comments at the top of the file
// for further (important!) details.
-// Create and destroy the singleton StatisticsRecorder object.
-void TelemetryHistogram::CreateStatisticsRecorder()
-{
- StaticMutexAutoLock locker(gTelemetryHistogramMutex);
- MOZ_ASSERT(!gStatisticsRecorder);
- gStatisticsRecorder = new base::StatisticsRecorder();
-}
-
-void TelemetryHistogram::DestroyStatisticsRecorder()
-{
- StaticMutexAutoLock locker(gTelemetryHistogramMutex);
- MOZ_ASSERT(gStatisticsRecorder);
- if (gStatisticsRecorder) {
- delete gStatisticsRecorder;
- gStatisticsRecorder = nullptr;
- }
-}
-
void TelemetryHistogram::InitializeGlobalState(bool canRecordBase,
bool canRecordExtended)
{
StaticMutexAutoLock locker(gTelemetryHistogramMutex);
MOZ_ASSERT(!gInitDone, "TelemetryHistogram::InitializeGlobalState "
"may only be called once");
gCanRecordBase = canRecordBase;
--- a/toolkit/components/telemetry/TelemetryHistogram.h
+++ b/toolkit/components/telemetry/TelemetryHistogram.h
@@ -14,19 +14,16 @@
// 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.
// For the public interface to Telemetry functionality, see Telemetry.h.
namespace TelemetryHistogram {
-void CreateStatisticsRecorder();
-void DestroyStatisticsRecorder();
-
void InitializeGlobalState(bool canRecordBase, bool canRecordExtended);
void DeInitializeGlobalState();
#ifdef DEBUG
bool GlobalStateHasBeenInitialized();
#endif
bool CanRecordBase();
void SetCanRecordBase(bool b);
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -4617,47 +4617,28 @@ void XRE_GlibInit()
// g_type_init (2.32 <= gLib version < 2.36)."
g_thread_init(nullptr);
g_type_init();
ran_once = true;
}
}
#endif
-// Separate stub function to let us specifically suppress it in Valgrind
-void
-XRE_CreateStatsObject()
-{
- // Initialize global variables used by histogram collection
- // machinery that is used by by Telemetry. Note: is never de-initialised.
- Telemetry::CreateStatisticsRecorder();
-}
-
/*
* XRE_main - A class based main entry point used by most platforms.
* Note that on OSX, aAppData->xreDirectory will point to
* .app/Contents/Resources.
*/
int
XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig)
{
ScopedLogging log;
mozilla::LogModule::Init();
- // NB: this must happen after the creation of |ScopedLogging log| since
- // ScopedLogging::ScopedLogging calls NS_LogInit, and
- // XRE_CreateStatsObject calls Telemetry::CreateStatisticsRecorder,
- // and NS_LogInit must be called before Telemetry::CreateStatisticsRecorder.
- // NS_LogInit must be called before Telemetry::CreateStatisticsRecorder
- // so as to avoid many log messages of the form
- // WARNING: XPCOM objects created/destroyed from static ctor/dtor: [..]
- // See bug 1279614.
- XRE_CreateStatsObject();
-
#if defined(MOZ_SANDBOX) && defined(XP_LINUX) && !defined(ANDROID)
SandboxInfo::ThreadingCheck();
#endif
#ifdef MOZ_CODE_COVERAGE
CodeCoverageHandler::Init();
#endif
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -75,18 +75,16 @@
#include "mozilla/ipc/XPCShellEnvironment.h"
#include "mozilla/WindowsDllBlocklist.h"
#include "GMPProcessChild.h"
#include "mozilla/gfx/GPUProcessImpl.h"
#include "GeckoProfiler.h"
-#include "mozilla/Telemetry.h"
-
#if defined(MOZ_SANDBOX) && defined(XP_WIN)
#include "mozilla/sandboxTarget.h"
#include "mozilla/sandboxing/loggingCallbacks.h"
#endif
#if defined(MOZ_CONTENT_SANDBOX)
#include "mozilla/SandboxSettings.h"
#if !defined(MOZ_WIDGET_GONK)
@@ -404,24 +402,16 @@ XRE_InitChildProcess(int aArgc,
SandboxTarget::Instance()->SetTargetServices(aChildData->sandboxTargetServices);
}
#endif
#endif
// NB: This must be called before profiler_init
ScopedLogging logger;
- // This is needed by Telemetry to initialize histogram collection.
- // NB: This must be called after NS_LogInit().
- // NS_LogInit must be called before Telemetry::CreateStatisticsRecorder
- // so as to avoid many log messages of the form
- // WARNING: XPCOM objects created/destroyed from static ctor/dtor: [..]
- // See bug 1279614.
- Telemetry::CreateStatisticsRecorder();
-
mozilla::LogModule::Init();
char aLocal;
AutoProfilerInit profilerInit(&aLocal);
AUTO_PROFILER_LABEL("XRE_InitChildProcess", OTHER);
// Ensure AbstractThread is minimally setup, so async IPC messages
@@ -725,17 +715,16 @@ XRE_InitChildProcess(int aArgc,
#if defined(XP_WIN) && !defined(DEBUG)
// XXX Bug 1320134: added for diagnosing the crashes because we're running out
// of TLS indices on Windows. Remove after the root cause is found.
if (XRE_GetProcessType() == GeckoProcessType_Content) {
mozilla::ShutdownTlsAllocationTracker();
}
#endif
- Telemetry::DestroyStatisticsRecorder();
return XRE_DeinitCommandLine();
}
MessageLoop*
XRE_GetIOMessageLoop()
{
if (sChildProcessType == GeckoProcessType_Default) {
return BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO);