From 486f94ee6a06be05ebbeac2cf3fc6dc912a4a1bf Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Tue, 6 Dec 2011 23:18:30 -0800 Subject: [PATCH] Back out 7341f4e8b3f3, d91429762579, d0a362467a96, 0aba56de1824 (bug 701863) for Windows timeouts in test_TelemetryPing.js --- toolkit/components/telemetry/Telemetry.cpp | 124 ++++-------------- toolkit/components/telemetry/TelemetryPing.js | 35 +++-- toolkit/components/telemetry/nsITelemetry.idl | 19 --- .../tests/unit/test_TelemetryPing.js | 17 +-- .../telemetry/tests/unit/test_nsITelemetry.js | 3 + 5 files changed, 43 insertions(+), 155 deletions(-) diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 5a62dba01c7..05754d19a80 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -83,8 +83,6 @@ public: private: bool AddSlowSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread); - // Like GetHistogramById, but returns the underlying C++ object, not the JS one. - nsresult GetHistogramByName(const nsACString &name, Histogram **ret); // This is used for speedy JS string->Telemetry::ID conversions typedef nsBaseHashtableET CharPtrEntryType; typedef nsTHashtable HistogramMapType; @@ -110,38 +108,17 @@ struct TelemetryHistogram { PRUint32 max; PRUint32 bucketCount; PRUint32 histogramType; - const char *comment; }; const TelemetryHistogram gHistograms[] = { -#define HISTOGRAM(id, min, max, bucket_count, histogram_type, comment) \ - { NULL, NS_STRINGIFY(id), min, max, bucket_count, \ - nsITelemetry::HISTOGRAM_ ## histogram_type, comment }, +#define HISTOGRAM(id, min, max, bucket_count, histogram_type, b) \ + { NULL, NS_STRINGIFY(id), min, max, bucket_count, nsITelemetry::HISTOGRAM_ ## histogram_type }, #include "TelemetryHistograms.h" #undef HISTOGRAM }; -bool -TelemetryHistogramType(Histogram *h, PRUint32 *result) -{ - switch (h->histogram_type()) { - case Histogram::HISTOGRAM: - *result = nsITelemetry::HISTOGRAM_EXPONENTIAL; - break; - case Histogram::LINEAR_HISTOGRAM: - *result = nsITelemetry::HISTOGRAM_LINEAR; - break; - case Histogram::BOOLEAN_HISTOGRAM: - *result = nsITelemetry::HISTOGRAM_BOOLEAN; - break; - default: - return false; - } - return true; -} - nsresult HistogramGet(const char *name, PRUint32 min, PRUint32 max, PRUint32 bucketCount, PRUint32 histogramType, Histogram **result) @@ -222,6 +199,7 @@ ReflectHistogramSnapshot(JSContext *cx, JSObject *obj, Histogram *h) && FillRanges(cx, rarray, h) && (counts_array = JS_NewArrayObject(cx, count, NULL)) && JS_DefineProperty(cx, obj, "counts", OBJECT_TO_JSVAL(counts_array), NULL, NULL, JSPROP_ENUMERATE) + && JS_DefineProperty(cx, obj, "static", static_histogram, NULL, NULL, JSPROP_ENUMERATE) )) { return JS_FALSE; } @@ -421,82 +399,6 @@ TelemetryImpl::AddSlowSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread) return true; } -NS_IMETHODIMP -TelemetryImpl::GetRegisteredHistograms(JSContext *cx, jsval *ret) -{ - size_t count = ArrayLength(gHistograms); - JSObject *info = JS_NewObject(cx, NULL, NULL, NULL); - if (!info) - return NS_ERROR_FAILURE; - - for (size_t i = 0; i < count; ++i) { - JSString *comment = JS_InternString(cx, gHistograms[i].comment); - - if (!(comment - && JS_DefineProperty(cx, info, gHistograms[i].id, - STRING_TO_JSVAL(comment), NULL, NULL, - JSPROP_ENUMERATE))) { - return NS_ERROR_FAILURE; - } - } - - *ret = OBJECT_TO_JSVAL(info); - return NS_OK; -} - -nsresult -TelemetryImpl::GetHistogramByName(const nsACString &name, Histogram **ret) -{ - // Cache names - // Note the histogram names are statically allocated - if (!mHistogramMap.Count()) { - for (PRUint32 i = 0; i < Telemetry::HistogramCount; i++) { - CharPtrEntryType *entry = mHistogramMap.PutEntry(gHistograms[i].id); - if (NS_UNLIKELY(!entry)) { - mHistogramMap.Clear(); - return NS_ERROR_OUT_OF_MEMORY; - } - entry->mData = (Telemetry::ID) i; - } - } - - CharPtrEntryType *entry = mHistogramMap.GetEntry(PromiseFlatCString(name).get()); - if (!entry) - return NS_ERROR_FAILURE; - - nsresult rv = GetHistogramByEnumId(entry->mData, ret); - if (NS_FAILED(rv)) - return rv; - - return NS_OK; -} - -NS_IMETHODIMP -TelemetryImpl::HistogramFrom(const nsACString &name, const nsACString &existing_name, - JSContext *cx, jsval *ret) -{ - Histogram *existing; - nsresult rv = GetHistogramByName(existing_name, &existing); - if (NS_FAILED(rv)) - return rv; - - PRUint32 histogramType; - bool success = TelemetryHistogramType(existing, &histogramType); - if (!success) - return NS_ERROR_INVALID_ARG; - - Histogram *clone; - rv = HistogramGet(PromiseFlatCString(name).get(), existing->declared_min(), - existing->declared_max(), existing->bucket_count(), - histogramType, &clone); - if (NS_FAILED(rv)) - return rv; - - Histogram::SampleSet ss; - existing->SnapshotSample(&ss); - clone->AddSampleSet(ss); - return WrapAndReturnHistogram(clone, cx, ret); -} NS_IMETHODIMP TelemetryImpl::GetHistogramSnapshots(JSContext *cx, jsval *ret) @@ -533,8 +435,26 @@ TelemetryImpl::GetHistogramSnapshots(JSContext *cx, jsval *ret) NS_IMETHODIMP TelemetryImpl::GetHistogramById(const nsACString &name, JSContext *cx, jsval *ret) { + // Cache names + // Note the histogram names are statically allocated + if (!mHistogramMap.Count()) { + for (PRUint32 i = 0; i < Telemetry::HistogramCount; i++) { + CharPtrEntryType *entry = mHistogramMap.PutEntry(gHistograms[i].id); + if (NS_UNLIKELY(!entry)) { + mHistogramMap.Clear(); + return NS_ERROR_OUT_OF_MEMORY; + } + entry->mData = (Telemetry::ID) i; + } + } + + CharPtrEntryType *entry = mHistogramMap.GetEntry(PromiseFlatCString(name).get()); + if (!entry) + return NS_ERROR_FAILURE; + Histogram *h; - nsresult rv = GetHistogramByName(name, &h); + + nsresult rv = GetHistogramByEnumId(entry->mData, &h); if (NS_FAILED(rv)) return rv; diff --git a/toolkit/components/telemetry/TelemetryPing.js b/toolkit/components/telemetry/TelemetryPing.js index 9b93ad67c35..e4d041b60f2 100644 --- a/toolkit/components/telemetry/TelemetryPing.js +++ b/toolkit/components/telemetry/TelemetryPing.js @@ -125,6 +125,7 @@ TelemetryPing.prototype = { _histograms: {}, _initialized: false, _prevValues: {}, + _sqliteOverhead: {}, /** * Returns a set of histograms that can be converted into JSON @@ -135,11 +136,19 @@ TelemetryPing.prototype = { */ getHistograms: function getHistograms() { let hls = Telemetry.histogramSnapshots; - let info = Telemetry.registeredHistograms; let ret = {}; - function processHistogram(name, hgram) { - let r = hgram.ranges;; + // bug 701583: report sqlite overhead on startup + for (let key in this._sqliteOverhead) { + hls[key] = this._sqliteOverhead[key]; + } + + for (let key in hls) { + let hgram = hls[key]; + if (!hgram.static) + continue; + + let r = hgram.ranges; let c = hgram.counts; let retgram = { range: [r[1], r[r.length - 1]], @@ -169,18 +178,8 @@ TelemetryPing.prototype = { // add an upper bound if (last && last < c.length) retgram.values[r[last]] = 0; - ret[name] = retgram; - }; - - for (let name in hls) { - if (info[name]) { - processHistogram(name, hls[name]); - let startup_name = "STARTUP_" + name; - if (hls[startup_name]) - processHistogram(startup_name, hls[startup_name]); - } + ret[key] = retgram; } - return ret; }, @@ -306,11 +305,11 @@ TelemetryPing.prototype = { * Make a copy of sqlite histograms on startup */ gatherStartupSqlite: function gatherStartupSqlite() { - let info = Telemetry.registeredHistograms; + let hls = Telemetry.histogramSnapshots; let sqlite_re = /SQLITE/; - for (let name in info) { - if (sqlite_re.test(name)) - Telemetry.histogramFrom("STARTUP_" + name, name); + for (let key in hls) { + if (sqlite_re.test(key)) + this._sqliteOverhead["STARTUP_" + key] = hls[key]; } }, diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index 43a38f87dd7..0cb2455efb4 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -66,14 +66,6 @@ interface nsITelemetry : nsISupports [implicit_jscontext] readonly attribute jsval histogramSnapshots; - /** - * An object whose properties are the names of histograms defined in - * TelemetryHistograms.h and whose corresponding values are the textual - * comments associated with said histograms. - */ - [implicit_jscontext] - readonly attribute jsval registeredHistograms; - /** * Create and return a histogram where bucket sizes increase exponentially. Parameters: * @@ -89,17 +81,6 @@ interface nsITelemetry : nsISupports [implicit_jscontext] jsval newHistogram(in ACString name, in PRUint32 min, in PRUint32 max, in PRUint32 bucket_count, in unsigned long histogram_type); - /** - * Create a histogram using the current state of an existing histogram. The - * existing histogram must be registered in TelemetryHistograms.h. - * - * @param name Unique histogram name - * @param existing_name Existing histogram name - * The returned object has the same functions as a histogram returned from newHistogram. - */ - [implicit_jscontext] - jsval histogramFrom(in ACString name, in ACString existing_name); - /** * Same as newHistogram above, but for histograms registered in TelemetryHistograms.h. * diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js index bb8e8b5bbc1..4ba4d31e3a7 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js @@ -15,16 +15,11 @@ Cu.import("resource://gre/modules/LightweightThemeManager.jsm"); const PATH = "/submit/telemetry/test-ping"; const SERVER = "http://localhost:4444"; const IGNORE_HISTOGRAM = "test::ignore_me"; -const HISTOGRAM_FOR_STARTUP = "PAGE_FAULTS_HARD" -const CLONED_STARTUP_HISTOGRAM = "STARTUP_PAGE_FAULTS_HARD" -const IGNORE_HISTOGRAM_TO_CLONE = "MEMORY_HEAP_ALLOCATED" -const IGNORE_CLONED_HISTOGRAM = "test::ignore_me_also" const BinaryInputStream = Components.Constructor( "@mozilla.org/binaryinputstream;1", "nsIBinaryInputStream", "setInputStream"); -const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); var httpserver = new nsHttpServer(); var gFinished = false; @@ -32,7 +27,6 @@ var gFinished = false; function telemetry_ping () { const TelemetryPing = Cc["@mozilla.org/base/telemetry-ping;1"].getService(Ci.nsIObserver); TelemetryPing.observe(null, "test-ping", SERVER); - TelemetryPing.observe(null, "sessionstore-windows-restored", null); } function nonexistentServerObserver(aSubject, aTopic, aData) { @@ -49,9 +43,8 @@ function nonexistentServerObserver(aSubject, aTopic, aData) { function telemetryObserver(aSubject, aTopic, aData) { Services.obs.removeObserver(telemetryObserver, aTopic); httpserver.registerPathHandler(PATH, checkHistograms); + const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); Telemetry.newHistogram(IGNORE_HISTOGRAM, 1, 2, 3, Telemetry.HISTOGRAM_BOOLEAN); - Telemetry.histogramFrom(CLONED_STARTUP_HISTOGRAM, HISTOGRAM_FOR_STARTUP); - Telemetry.histogramFrom(IGNORE_CLONED_HISTOGRAM, IGNORE_HISTOGRAM_TO_CLONE); Services.startup.interrupted = true; telemetry_ping(); } @@ -84,15 +77,7 @@ function checkHistograms(request, response) { const TELEMETRY_PING = "TELEMETRY_PING"; const TELEMETRY_SUCCESS = "TELEMETRY_SUCCESS"; do_check_true(TELEMETRY_PING in payload.histograms); - do_check_true(CLONED_STARTUP_HISTOGRAM in payload.histograms); - let rh = Telemetry.registeredHistograms; - for (let name in rh) { - if (/SQLITE/.test(name) && name in payload.histograms) { - do_check_true(("STARTUP_" + name) in payload.histograms); - } - } do_check_false(IGNORE_HISTOGRAM in payload.histograms); - do_check_false(IGNORE_CLONED_HISTOGRAM in payload.histograms); // There should be one successful report from the previous telemetry ping. const expected_tc = { diff --git a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js index 8909361bcce..f0485dd07da 100644 --- a/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js +++ b/toolkit/components/telemetry/tests/unit/test_nsITelemetry.js @@ -33,6 +33,8 @@ function test_histogram(histogram_type, name, min, max, bucket_count) { do_check_eq(gh.min, min) do_check_eq(gh.max, max) + do_check_false(gh.static); + // Check that booleans work with nonboolean histograms h.add(false); h.add(true); @@ -86,6 +88,7 @@ function test_getHistogramById() { do_check_eq(s.histogram_type, Telemetry.HISTOGRAM_EXPONENTIAL); do_check_eq(s.min, 1); do_check_eq(s.max, 10000); + do_check_true(s.static); } // Check that telemetry doesn't record in private mode