Bug 1448945 - Enable recording events in artifact builds without rebuilding Firefox. r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Mon, 09 Apr 2018 15:04:49 +0200
changeset 414321 03ec35311b196fb3f4e960daed451207e866a7de
parent 414299 697d0f7076eb03cc8a5f18856616d80e5ebf9a55
child 414322 4badb094b348d6939e946983e68fecf7f43b9e9a
push id102317
push userbtara@mozilla.com
push dateWed, 18 Apr 2018 22:47:42 +0000
treeherdermozilla-inbound@c8b6fba2ae94 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1448945
milestone61.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 1448945 - Enable recording events in artifact builds without rebuilding Firefox. r=chutten This patch enables the Firefox Telemetry core to register and record to dynamic builtin scalars. MozReview-Commit-ID: 8FeAgmmsQXw
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/TelemetryEvent.cpp
toolkit/components/telemetry/TelemetryEvent.h
toolkit/components/telemetry/nsITelemetry.idl
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -1810,17 +1810,25 @@ TelemetryImpl::SnapshotEvents(uint32_t a
   return TelemetryEvent::CreateSnapshots(aDataset, aClear, aCx, optional_argc, aResult);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::RegisterEvents(const nsACString& aCategory,
                               JS::Handle<JS::Value> aEventData,
                               JSContext* cx)
 {
-  return TelemetryEvent::RegisterEvents(aCategory, aEventData, cx);
+  return TelemetryEvent::RegisterEvents(aCategory, aEventData, false, cx);
+}
+
+NS_IMETHODIMP
+TelemetryImpl::RegisterBuiltinEvents(const nsACString& aCategory,
+                              JS::Handle<JS::Value> aEventData,
+                              JSContext* cx)
+{
+  return TelemetryEvent::RegisterEvents(aCategory, aEventData, true, cx);
 }
 
 NS_IMETHODIMP
 TelemetryImpl::ClearEvents()
 {
   TelemetryEvent::ClearEvents();
   return NS_OK;
 }
--- a/toolkit/components/telemetry/TelemetryEvent.cpp
+++ b/toolkit/components/telemetry/TelemetryEvent.cpp
@@ -123,32 +123,34 @@ typedef nsClassHashtable<nsCStringHashKe
 struct EventKey {
   uint32_t id;
   bool dynamic;
 };
 
 struct DynamicEventInfo {
   DynamicEventInfo(const nsACString& category, const nsACString& method,
                    const nsACString& object, nsTArray<nsCString>& extra_keys,
-                   bool recordOnRelease)
+                   bool recordOnRelease, bool builtin)
     : category(category)
     , method(method)
     , object(object)
     , extra_keys(extra_keys)
     , recordOnRelease(recordOnRelease)
+    , builtin(builtin)
   {}
 
   DynamicEventInfo(const DynamicEventInfo&) = default;
   DynamicEventInfo& operator=(const DynamicEventInfo&) = delete;
 
   const nsCString category;
   const nsCString method;
   const nsCString object;
   const nsTArray<nsCString> extra_keys;
   const bool recordOnRelease;
+  const bool builtin;
 
   size_t
   SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
     size_t n = 0;
 
     n += category.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
     n += method.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
@@ -311,18 +313,24 @@ nsClassHashtable<nsCStringHashKey, Event
 // The CategoryName -> CategoryID cache map.
 StringUintMap gCategoryNameIDMap;
 
 // This tracks the IDs of the categories for which recording is enabled.
 nsTHashtable<nsCStringHashKey> gEnabledCategories;
 
 // The main event storage. Events are inserted here, keyed by process id and
 // in recording order.
+typedef nsUint32HashKey ProcessIDHashKey;
 typedef nsTArray<EventRecord> EventRecordArray;
-nsClassHashtable<nsUint32HashKey, EventRecordArray> gEventRecords;
+typedef nsClassHashtable<ProcessIDHashKey, EventRecordArray> EventRecordsMapType;
+
+EventRecordsMapType gEventRecords;
+// Provide separate storage for "dynamic builtin" events needed to
+// support "build faster" in local developer builds.
+EventRecordsMapType gBuiltinEventRecords;
 
 // The details on dynamic events that are recorded from addons are registered here.
 StaticAutoPtr<nsTArray<DynamicEventInfo>> gDynamicEventInfo;
 
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
@@ -387,22 +395,27 @@ CanRecordEvent(const StaticMutexAutoLock
 bool
 IsExpired(const EventKey& key)
 {
   return key.id == kExpiredEventId;
 }
 
 EventRecordArray*
 GetEventRecordsForProcess(const StaticMutexAutoLock& lock, ProcessID processType,
-                          const EventKey& eventKey)
+                          const EventKey& eventKey, bool isDynamicBuiltin)
 {
+  // Put dynamic-builtin events (used to support "build faster") in a
+  // separate storage.
+  EventRecordsMapType& processStorage =
+    isDynamicBuiltin ? gBuiltinEventRecords : gEventRecords;
+
   EventRecordArray* eventRecords = nullptr;
-  if (!gEventRecords.Get(uint32_t(processType), &eventRecords)) {
+  if (!processStorage.Get(uint32_t(processType), &eventRecords)) {
     eventRecords = new EventRecordArray();
-    gEventRecords.Put(uint32_t(processType), eventRecords);
+    processStorage.Put(uint32_t(processType), eventRecords);
   }
   return eventRecords;
 }
 
 EventKey*
 GetEventKey(const StaticMutexAutoLock& lock, const nsACString& category,
            const nsACString& method, const nsACString& object)
 {
@@ -446,35 +459,40 @@ RecordEvent(const StaticMutexAutoLock& l
             const Maybe<nsCString>& value, const ExtraArray& extra)
 {
   // Look up the event id.
   EventKey* eventKey = GetEventKey(lock, category, method, object);
   if (!eventKey) {
     return RecordEventResult::UnknownEvent;
   }
 
-  if (eventKey->dynamic) {
-    processType = ProcessID::Dynamic;
-  }
-
-  EventRecordArray* eventRecords = GetEventRecordsForProcess(lock, processType, *eventKey);
-
-  // Apply hard limit on event count in storage.
-  if (eventRecords->Length() >= kMaxEventRecords) {
-    return RecordEventResult::StorageLimitReached;
-  }
-
   // If the event is expired or not enabled for this process, we silently drop this call.
   // We don't want recording for expired probes to be an error so code doesn't
   // have to be removed at a specific time or version.
   // Even logging warnings would become very noisy.
   if (IsExpired(*eventKey)) {
     return RecordEventResult::ExpiredEvent;
   }
 
+  // Fixup the process id only for non-builtin (e.g. supporting build faster)
+  // dynamic events.
+  if (eventKey->dynamic &&
+      !(*gDynamicEventInfo)[eventKey->id].builtin) {
+    processType = ProcessID::Dynamic;
+  }
+
+  bool isDynamicBuiltin = eventKey->dynamic && (*gDynamicEventInfo)[eventKey->id].builtin;
+  EventRecordArray* eventRecords =
+    GetEventRecordsForProcess(lock, processType, *eventKey, isDynamicBuiltin);
+
+  // Apply hard limit on event count in storage.
+  if (eventRecords->Length() >= kMaxEventRecords) {
+    return RecordEventResult::StorageLimitReached;
+  }
+
   // Check whether the extra keys passed are valid.
   if (!CheckExtraKeysValid(*eventKey, extra)) {
     return RecordEventResult::InvalidExtraKey;
   }
 
   // Check whether we can record this event.
   if (!CanRecordEvent(lock, *eventKey, processType)) {
     return RecordEventResult::Ok;
@@ -727,16 +745,17 @@ TelemetryEvent::DeInitializeGlobalState(
 
   gCanRecordBase = false;
   gCanRecordExtended = false;
 
   gEventNameIDMap.Clear();
   gCategoryNameIDMap.Clear();
   gEnabledCategories.Clear();
   gEventRecords.Clear();
+  gBuiltinEventRecords.Clear();
 
   gDynamicEventInfo = nullptr;
 
   gInitDone = false;
 }
 
 void
 TelemetryEvent::SetCanRecordBase(bool b)
@@ -951,16 +970,17 @@ GetArrayPropertyValues(JSContext* cx, JS
   }
 
   return true;
 }
 
 nsresult
 TelemetryEvent::RegisterEvents(const nsACString& aCategory,
                                JS::Handle<JS::Value> aEventData,
+                               bool aBuiltin,
                                JSContext* cx)
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Events can only be registered in the parent process");
 
   if (!IsValidIdentifierString(aCategory, 30, true, false)) {
     JS_ReportErrorASCII(cx, "Category parameter should match the identifier pattern.");
     return NS_ERROR_INVALID_ARG;
@@ -1072,17 +1092,17 @@ TelemetryEvent::RegisterEvents(const nsA
     }
 
     // Append event infos to be registered.
     for (auto& method : methods) {
       for (auto& object : objects) {
         // We defer the actual registration here in case any other event description is invalid.
         // In that case we don't need to roll back any partial registration.
         DynamicEventInfo info{aCategory, method, object,
-                              extra_keys, recordOnRelease};
+                              extra_keys, recordOnRelease, aBuiltin};
         newEventInfos.AppendElement(info);
         newEventExpired.AppendElement(expired);
       }
     }
   }
 
   {
     StaticMutexAutoLock locker(gTelemetryEventsMutex);
--- a/toolkit/components/telemetry/TelemetryEvent.h
+++ b/toolkit/components/telemetry/TelemetryEvent.h
@@ -31,17 +31,17 @@ void SetCanRecordExtended(bool b);
 // JS API Endpoints.
 nsresult RecordEvent(const nsACString& aCategory, const nsACString& aMethod,
                      const nsACString& aObject, JS::HandleValue aValue,
                      JS::HandleValue aExtra, JSContext* aCx,
                      uint8_t optional_argc);
 
 void SetEventRecordingEnabled(const nsACString& aCategory, bool aEnabled);
 nsresult RegisterEvents(const nsACString& aCategory, JS::Handle<JS::Value> aEventData,
-                        JSContext* cx);
+                        bool aBuiltin, JSContext* cx);
 
 nsresult CreateSnapshots(uint32_t aDataset, bool aClear, JSContext* aCx,
                          uint8_t optional_argc, JS::MutableHandleValue aResult);
 
 // Record events from child processes.
 nsresult RecordChildEvents(mozilla::Telemetry::ProcessID aProcessType,
                            const nsTArray<mozilla::Telemetry::ChildEventData>& aEvents);
 
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -511,16 +511,27 @@ interface nsITelemetry : nsISupports
    *                                            Defaults to false.
    * @param aEventData.<name>.expired Optional, whether this event entry is expired. This allows
    *                                  recording it without error, but it will be discarded. Defaults to false.
    */
   [implicit_jscontext]
   void registerEvents(in ACString aCategory, in jsval aEventData);
 
   /**
+   * Parent process only. Register dynamic builtin events. The parameters
+   * have the same meaning as the usual |registerEvents| function.
+   *
+   * This function is only meant to be used to support the "artifact build"/
+   * "build faster" developers by allowing to add new events without rebuilding
+   * the C++ components including the headers files.
+  */
+  [implicit_jscontext]
+  void registerBuiltinEvents(in ACString aCategory, in jsval aEventData);
+
+  /**
    * Parent process only. Register new scalars to record them from addons. This
    * allows registering multiple scalars for a category. They will be valid only for
    * the current Firefox session.
    * Note that scalars shipping in Firefox should be registered in Scalars.yaml.
    *
    * @param aCategoryName The unique category the scalars are registered in.
    * @param aScalarData An object that contains registration data for multiple scalars in the form:
    *   {
@@ -542,17 +553,17 @@ interface nsITelemetry : nsISupports
   [implicit_jscontext]
   void registerScalars(in ACString aCategoryName, in jsval aScalarData);
 
   /**
    * Parent process only. Register dynamic builtin scalars. The parameters
    * have the same meaning as the usual |registerScalars| function.
    *
    * This function is only meant to be used to support the "artifact build"/
-   * "built faster" developers by allowing to add new scalars without rebuilding
+   * "build faster" developers by allowing to add new scalars without rebuilding
    * the C++ components including the headers files.
    */
   [implicit_jscontext]
   void registerBuiltinScalars(in ACString aCategoryName, in jsval aScalarData);
 
   /**
    * Resets all the stored events. This is intended to be only used in tests.
    */