Bug 1443587 - part 5 - more efficient string serialization; r=chutten
authorNathan Froyd <froydnj@mozilla.com>
Wed, 07 Mar 2018 11:00:44 -0500
changeset 461985 5c78a8fff848584e3fa97628a6c2c11004ba69b7
parent 461984 dbf8c7f084e131f9307998690add39bedea3a2bd
child 462088 97d8b867071938df9e8e2126d986ea348f55c649
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1443587
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1443587 - part 5 - more efficient string serialization; r=chutten The code for serializing an EventKey in SerializeEventsArray does something peculiar: it creates a temporary array of nsCStrings, copies the appropriate strings from the event info for the key, and then converts those into JavaScript strings. But we shouldn't need to do any copying into the temporary array. We can do everything directly on the strings from the event info themselves. It seemed best to introduce a ToJSString for this purpose, which comes in handy in a couple other places as well.
toolkit/components/telemetry/TelemetryCommon.cpp
toolkit/components/telemetry/TelemetryCommon.h
toolkit/components/telemetry/TelemetryEvent.cpp
toolkit/components/telemetry/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/TelemetryCommon.cpp
+++ b/toolkit/components/telemetry/TelemetryCommon.cpp
@@ -165,11 +165,18 @@ IsValidIdentifierString(const nsACString
                              aAllowInfixUnderscore && infix)) {
         return false;
       }
   }
 
   return true;
 }
 
+JSString*
+ToJSString(JSContext* cx, const nsACString& aStr)
+{
+  const NS_ConvertUTF8toUTF16 wide(aStr);
+  return JS_NewUCStringCopyN(cx, wide.Data(), wide.Length());
+}
+
 } // namespace Common
 } // namespace Telemetry
 } // namespace mozilla
--- a/toolkit/components/telemetry/TelemetryCommon.h
+++ b/toolkit/components/telemetry/TelemetryCommon.h
@@ -103,13 +103,24 @@ GeckoProcessType GetGeckoProcessType(Pro
  * @param aAllowInfixPeriod Whether or not to allow infix dots.
  * @param aAllowInfixUnderscore Whether or not to allow infix underscores.
  * @returns true if the string validates correctly, false otherwise.
  */
 bool
 IsValidIdentifierString(const nsACString& aStr, const size_t aMaxLength,
                         const bool aAllowInfixPeriod, const bool aAllowInfixUnderscore);
 
+/**
+ * Convert the given UTF8 string to a JavaScript string.  The returned
+ * string's contents will be the UTF16 conversion of the given string.
+ *
+ * @param cx The JS context.
+ * @param aStr The UTF8 string.
+ * @returns a JavaScript string.
+ */
+JSString*
+ToJSString(JSContext* cx, const nsACString& aStr);
+
 } // namespace Common
 } // namespace Telemetry
 } // namespace mozilla
 
 #endif // TelemetryCommon_h__
--- a/toolkit/components/telemetry/TelemetryEvent.cpp
+++ b/toolkit/components/telemetry/TelemetryEvent.cpp
@@ -39,16 +39,17 @@ using mozilla::Telemetry::Common::AutoHa
 using mozilla::Telemetry::Common::IsExpiredVersion;
 using mozilla::Telemetry::Common::CanRecordDataset;
 using mozilla::Telemetry::Common::IsInDataset;
 using mozilla::Telemetry::Common::MsSinceProcessStart;
 using mozilla::Telemetry::Common::LogToBrowserConsole;
 using mozilla::Telemetry::Common::CanRecordInProcess;
 using mozilla::Telemetry::Common::GetNameForProcessID;
 using mozilla::Telemetry::Common::IsValidIdentifierString;
+using mozilla::Telemetry::Common::ToJSString;
 using mozilla::Telemetry::EventExtraEntry;
 using mozilla::Telemetry::ChildEventData;
 using mozilla::Telemetry::ProcessID;
 
 namespace TelemetryIPCAccumulator = mozilla::TelemetryIPCAccumulator;
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
@@ -572,44 +573,46 @@ SerializeEventsArray(const EventRecordAr
 
     // Add timestamp.
     JS::Rooted<JS::Value> val(cx);
     if (!items.append(JS::NumberValue(floor(record.Timestamp())))) {
       return NS_ERROR_FAILURE;
     }
 
     // Add category, method, object.
-    nsCString strings[3];
+    auto addCategoryMethodObjectValues = [&](const nsACString& category,
+                                             const nsACString& method,
+                                             const nsACString& object) -> bool {
+      return items.append(JS::StringValue(ToJSString(cx, category))) &&
+             items.append(JS::StringValue(ToJSString(cx, method))) &&
+             items.append(JS::StringValue(ToJSString(cx, object)));
+    };
+      
     const EventKey& eventKey = record.GetEventKey();
     if (!eventKey.dynamic) {
       const EventInfo& info = gEventInfo[eventKey.id];
-      strings[0] = info.common_info.category();
-      strings[1] = info.method();
-      strings[2] = info.object();
+      if (!addCategoryMethodObjectValues(info.common_info.category(),
+                                         info.method(),
+                                         info.object())) {
+        return NS_ERROR_FAILURE;
+      }
     } else if (gDynamicEventInfo) {
       const DynamicEventInfo& info = (*gDynamicEventInfo)[eventKey.id];
-      strings[0] = info.category;
-      strings[1] = info.method;
-      strings[2] = info.object;
-    }
-
-    for (const nsCString& s : strings) {
-      const NS_ConvertUTF8toUTF16 wide(s);
-      if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) {
+      if (!addCategoryMethodObjectValues(info.category, info.method,
+                                         info.object)) {
         return NS_ERROR_FAILURE;
       }
     }
 
     // Add the optional string value only when needed.
     // When the value field is empty and extra is not set, we can save a little space that way.
     // We still need to submit a null value if extra is set, to match the form:
     // [ts, category, method, object, null, extra]
     if (record.Value()) {
-      const NS_ConvertUTF8toUTF16 wide(record.Value().value());
-      if (!items.append(JS::StringValue(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length())))) {
+      if (!items.append(JS::StringValue(ToJSString(cx, record.Value().value())))) {
         return NS_ERROR_FAILURE;
       }
     } else if (!record.Extra().IsEmpty()) {
       if (!items.append(JS::NullValue())) {
         return NS_ERROR_FAILURE;
       }
     }
 
@@ -619,19 +622,18 @@ SerializeEventsArray(const EventRecordAr
       JS::RootedObject obj(cx, JS_NewPlainObject(cx));
       if (!obj) {
         return NS_ERROR_FAILURE;
       }
 
       // Add extra key & value entries.
       const ExtraArray& extra = record.Extra();
       for (uint32_t i = 0; i < extra.Length(); ++i) {
-        const NS_ConvertUTF8toUTF16 wide(extra[i].value);
         JS::Rooted<JS::Value> value(cx);
-        value.setString(JS_NewUCStringCopyN(cx, wide.Data(), wide.Length()));
+        value.setString(ToJSString(cx, extra[i].value));
 
         if (!JS_DefineProperty(cx, obj, extra[i].key.get(), value, JSPROP_ENUMERATE)) {
           return NS_ERROR_FAILURE;
         }
       }
       val.setObject(*obj);
 
       if (!items.append(val)) {
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -45,16 +45,17 @@ using mozilla::Telemetry::ProcessID;
 using mozilla::Telemetry::HistogramCount;
 using mozilla::Telemetry::Common::LogToBrowserConsole;
 using mozilla::Telemetry::Common::RecordedProcessType;
 using mozilla::Telemetry::Common::AutoHashtable;
 using mozilla::Telemetry::Common::GetNameForProcessID;
 using mozilla::Telemetry::Common::IsExpiredVersion;
 using mozilla::Telemetry::Common::CanRecordDataset;
 using mozilla::Telemetry::Common::IsInDataset;
+using mozilla::Telemetry::Common::ToJSString;
 
 namespace TelemetryIPCAccumulator = mozilla::TelemetryIPCAccumulator;
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // Naming: there are two kinds of functions in this file:
 //
@@ -879,18 +880,17 @@ KeyedHistogram::GetJSKeys(JSContext* cx,
 {
   JS::AutoValueVector keys(cx);
   if (!keys.reserve(mHistogramMap.Count())) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   for (auto iter = mHistogramMap.Iter(); !iter.Done(); iter.Next()) {
     JS::RootedValue jsKey(cx);
-    const NS_ConvertUTF8toUTF16 key(iter.Get()->GetKey());
-    jsKey.setString(JS_NewUCStringCopyN(cx, key.Data(), key.Length()));
+    jsKey.setString(ToJSString(cx, iter.Get()->GetKey()));
     if (!keys.append(jsKey)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
   JS::RootedObject jsKeys(cx, JS_NewArrayObject(cx, keys));
   if (!jsKeys) {
     return NS_ERROR_FAILURE;