Bug 709406: Fix up slow SQL reporting. r=taras. a=ehsan.

--HG--
extra : rebase_source : 09b9577733a4071d6abc778adad3f732446332a5

toolkit/components/telemetry/Telemetry.cpp

@@ -21,6 +21,7 @@
  *
  * Contributor(s):
  *   Taras Glek <tglek@mozilla.com>
+ *   Vladan Djeric <vdjeric@mozilla.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -81,7 +82,7 @@ public:
   typedef nsBaseHashtableET<nsCStringHashKey, StmtStats> SlowSQLEntryType;
 
 private:
-  bool AddSlowSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread);
+  bool AddSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread);
 
   // This is used for speedy JS string->Telemetry::ID conversions
   typedef nsBaseHashtableET<nsCharPtrHashKey, Telemetry::ID> CharPtrEntryType;
@@ -325,41 +326,26 @@ TelemetryImpl::NewHistogram(const nsACString &name, PRUint32 min, PRUint32 max,
 struct EnumeratorArgs {
   JSContext *cx;
   JSObject *statsObj;
-  JSObject *statementsObj;
-  int32 statementIndex;
 };
 
 PLDHashOperator
 StatementEnumerator(TelemetryImpl::SlowSQLEntryType *entry, void *arg)
 {
   EnumeratorArgs *args = static_cast<EnumeratorArgs *>(arg);
-  const nsACString &statement = entry->GetKey();
+  const nsACString &sql = entry->GetKey();
   jsval hitCount = UINT_TO_JSVAL(entry->mData.hitCount);
   jsval totalTime = UINT_TO_JSVAL(entry->mData.totalTime);
-  args->statementIndex++;
 
-  JSObject *hitsAndTimeObj = JS_NewArrayObject(args->cx, 2, nsnull);
-  if (!hitsAndTimeObj ||
-      !JS_SetElement(args->cx, hitsAndTimeObj, 0, &hitCount) ||
-      !JS_SetElement(args->cx, hitsAndTimeObj, 1, &totalTime))
+  JSObject *arrayObj = JS_NewArrayObject(args->cx, 2, nsnull);
+  if (!arrayObj ||
+      !JS_SetElement(args->cx, arrayObj, 0, &hitCount) ||
+      !JS_SetElement(args->cx, arrayObj, 1, &totalTime))
     return PL_DHASH_STOP;
 
-  jsid propertyId = INT_TO_JSID(args->statementIndex);
-  JSBool success = JS_DefinePropertyById(args->cx, args->statsObj,
-                                         INT_TO_JSID(args->statementIndex),
-                                         OBJECT_TO_JSVAL(hitsAndTimeObj),
-                                         NULL, NULL, JSPROP_ENUMERATE);
-  if (!success)
-    return PL_DHASH_STOP;
-
-  JSString *string = JS_NewStringCopyN(args->cx, statement.BeginReading(), statement.Length());
-  if (!string)
-    return PL_DHASH_STOP;
-
-  success = JS_DefinePropertyById(args->cx, args->statementsObj,
-                                  INT_TO_JSID(args->statementIndex),
-                                  STRING_TO_JSVAL(string), NULL, NULL,
-                                  JSPROP_ENUMERATE);
+  JSBool success = JS_DefineProperty(args->cx, args->statsObj,
+                                     sql.BeginReading(),
+                                     OBJECT_TO_JSVAL(arrayObj),
+                                     NULL, NULL, JSPROP_ENUMERATE);
   if (!success)
     return PL_DHASH_STOP;
 
@@ -367,28 +353,20 @@ StatementEnumerator(TelemetryImpl::SlowSQLEntryType *entry, void *arg)
 }
 
 bool
-TelemetryImpl::AddSlowSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread)
+TelemetryImpl::AddSQLInfo(JSContext *cx, JSObject *rootObj, bool mainThread)
 {
-  JSObject *statementsObj = JS_NewObject(cx, NULL, NULL, NULL);
   JSObject *statsObj = JS_NewObject(cx, NULL, NULL, NULL);
-  if (!statementsObj || !statsObj)
+  if (!statsObj)
     return false;
 
   JSBool ok = JS_DefineProperty(cx, rootObj,
-                                mainThread ? "slowSQLOnMain" : "slowSQLOnOther",
-                                OBJECT_TO_JSVAL(statementsObj),
+                                mainThread ? "mainThread" : "otherThreads",
+                                OBJECT_TO_JSVAL(statsObj),
                                 NULL, NULL, JSPROP_ENUMERATE);
   if (!ok)
     return false;
 
-  ok = JS_DefineProperty(cx, rootObj,
-                         mainThread ? "slowSQLStatsMain" : "slowSQLStatsOther",
-                         OBJECT_TO_JSVAL(statsObj),
-                         NULL, NULL, JSPROP_ENUMERATE);
-  if (!ok)
-    return false;
-
-  EnumeratorArgs args = { cx, statsObj, statementsObj, 0 };
+  EnumeratorArgs args = { cx, statsObj };
   nsTHashtable<SlowSQLEntryType> *sqlMap;
   sqlMap = (mainThread ? &mSlowSQLOnMainThread : &mSlowSQLOnOtherThread);
   PRUint32 num = sqlMap->EnumerateEntries(StatementEnumerator,
@@ -420,13 +398,23 @@ TelemetryImpl::GetHistogramSnapshots(JSContext *cx, jsval *ret)
       return NS_ERROR_FAILURE;
     }
   }
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+TelemetryImpl::GetSlowSQL(JSContext *cx, jsval *ret)
+{
+  JSObject *root_obj = JS_NewObject(cx, NULL, NULL, NULL);
+  if (!root_obj)
+    return NS_ERROR_FAILURE;
+  *ret = OBJECT_TO_JSVAL(root_obj);
 
   MutexAutoLock hashMutex(mHashMutex);
   // Add info about slow SQL queries on the main thread
-  if (!AddSlowSQLInfo(cx, root_obj, true))
+  if (!AddSQLInfo(cx, root_obj, true))
     return NS_ERROR_FAILURE;
   // Add info about slow SQL queries on other threads
-  if (!AddSlowSQLInfo(cx, root_obj, false))
+  if (!AddSQLInfo(cx, root_obj, false))
     return NS_ERROR_FAILURE;
 
   return NS_OK;

toolkit/components/telemetry/TelemetryPing.js

@@ -18,6 +18,7 @@
  *
  * Contributor(s):
  *   Taras Glek <tglek@mozilla.com>
+ *   Vladan Djeric <vdjeric@mozilla.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -323,7 +324,8 @@ TelemetryPing.prototype = {
       ver: PAYLOAD_VERSION,
       info: this.getMetadata(reason),
       simpleMeasurements: getSimpleMeasurements(),
-      histograms: this.getHistograms()
+      histograms: this.getHistograms(),
+      slowSQL: Telemetry.slowSQL
     };
 
     let isTestPing = (reason == "test-ping");

toolkit/components/telemetry/nsITelemetry.idl

@@ -21,6 +21,7 @@
  *
  * Contributor(s):
  *   Taras Glek <tglek@mozilla.com>
+ *   Vladan Djeric <vdjeric@mozilla.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -66,6 +67,24 @@ interface nsITelemetry : nsISupports
   [implicit_jscontext]
   readonly attribute jsval histogramSnapshots;
 
+  /*
+   * An object containing information about slow SQL prepared statements.
+   *
+   * {
+   *   mainThread: { "sqlString1": [<hit count>, <total time>], "sqlString2": [...], ... },
+   *   otherThreads: { "sqlString3": [<hit count>, <total time>], "sqlString4": [...], ... }
+   * }
+   *
+   * where:
+   *   mainThread: Slow statements that executed on the main thread
+   *   otherThreads: Slow statements that executed on a non-main thread
+   *   sqlString - String of the offending prepared statement
+   *   hit count - The number of times this statement required longer than the threshold time to execute
+   *   total time - The sum of all execution times above the threshold time for this statement
+   */
+  [implicit_jscontext]
+  readonly attribute jsval slowSQL;
+
   /** 
    * Create and return a histogram where bucket sizes increase exponentially. Parameters:
    *

toolkit/components/telemetry/tests/unit/test_TelemetryPing.js

@@ -90,6 +90,9 @@ function checkHistograms(request, response) {
   let tc = payload.histograms[TELEMETRY_SUCCESS]
   do_check_eq(uneval(tc), 
               uneval(expected_tc));
+
+  do_check_true(("mainThread" in payload.slowSQL) &&
+                ("otherThreads" in payload.slowSQL));
   gFinished = true;
 }
 

toolkit/components/telemetry/tests/unit/test_nsITelemetry.js

@@ -91,6 +91,11 @@ function test_getHistogramById() {
   do_check_true(s.static);
 }
 
+function test_getSlowSQL() {
+  var slow = Telemetry.slowSQL;
+  do_check_true(("mainThread" in slow) && ("otherThreads" in slow));
+}
+
 // Check that telemetry doesn't record in private mode
 function test_privateMode() {
   var h = Telemetry.newHistogram("test::private_mode_boolean", 1,2,3, Telemetry.HISTOGRAM_BOOLEAN);
@@ -117,5 +122,6 @@ function run_test()
 
   test_boolean_histogram();
   test_getHistogramById();
+  test_getSlowSQL();
   test_privateMode();
 }