Hoist responsibility for clearMarks/Measures to NativePerformanceObse…

…rver (#36312) Summary: Pull Request resolved: #36312 ## Changelog: [Internal] - `clearMarks` and `clearMeasures` methods are incidental to the `NativePerformance` TurboModule functionality, as in reality this responsibility belongs more on the `NativePerformanceObserver` and `PerformanceEntryReporter` side. This is something that [the standard indirectly suggests](https://www.w3.org/TR/user-timing/#clearmarks-method) as well (referencing [performance entry buffer](https://www.w3.org/TR/performance-timeline/#dfn-performance-entry-buffer)). The new implementation should be also a little bit more efficient, as it avoids calling the predicate for each entry. Finally (and frankly, the main reason for this change, from my perspective), it will simplify mocking/testing the JS part of the PerfAPI code. Reviewed By: rubennorte Differential Revision: D43621174 fbshipit-source-id: c4217a0da1d8ecbce797240627f7b4f057d85b97
facebook · Feb 28, 2023 · 14ab76a · 14ab76a
1 parent 5112bc5
commit 14ab76a
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 69 deletions.
diff --git a/Libraries/WebPerformance/NativePerformance.cpp b/Libraries/WebPerformance/NativePerformance.cpp
@@ -32,12 +32,6 @@ void NativePerformance::mark(
   PerformanceEntryReporter::getInstance().mark(name, startTime, duration);
 }
 
-void NativePerformance::clearMarks(
-    jsi::Runtime &rt,
-    std::optional<std::string> markName) {
-  PerformanceEntryReporter::getInstance().clearMarks(markName);
-}
-
 void NativePerformance::measure(
     jsi::Runtime &rt,
     std::string name,
@@ -50,12 +44,6 @@ void NativePerformance::measure(
       name, startTime, endTime, duration, startMark, endMark);
 }
 
-void NativePerformance::clearMeasures(
-    jsi::Runtime &rt,
-    std::optional<std::string> measureName) {
-  PerformanceEntryReporter::getInstance().clearMeasures(measureName);
-}
-
 std::unordered_map<std::string, double> NativePerformance::getSimpleMemoryInfo(
     jsi::Runtime &rt) {
   auto heapInfo = rt.instrumentation().getHeapInfo(false);

diff --git a/Libraries/WebPerformance/NativePerformance.h b/Libraries/WebPerformance/NativePerformance.h
@@ -27,7 +27,6 @@ class NativePerformance : public NativePerformanceCxxSpec<NativePerformance>,
 
   void
   mark(jsi::Runtime &rt, std::string name, double startTime, double duration);
-  void clearMarks(jsi::Runtime &rt, std::optional<std::string> markName);
 
   void measure(
       jsi::Runtime &rt,
@@ -37,7 +36,6 @@ class NativePerformance : public NativePerformanceCxxSpec<NativePerformance>,
       std::optional<double> duration,
       std::optional<std::string> startMark,
       std::optional<std::string> endMark);
-  void clearMeasures(jsi::Runtime &rt, std::optional<std::string> measureName);
 
   // To align with web API, we will make sure to return three properties
   // (jsHeapSizeLimit, totalJSHeapSize, usedJSHeapSize) + anything needed from

diff --git a/Libraries/WebPerformance/NativePerformance.js b/Libraries/WebPerformance/NativePerformance.js
@@ -16,8 +16,6 @@ export type NativeMemoryInfo = {[key: string]: number};
 
 export interface Spec extends TurboModule {
   +mark: (name: string, startTime: number, duration: number) => void;
-  +clearMarks: (markName?: string) => void;
-
   +measure: (
     name: string,
     startTime: number,
@@ -26,7 +24,6 @@ export interface Spec extends TurboModule {
     startMark?: string,
     endMark?: string,
   ) => void;
-  +clearMeasures: (measureName?: string) => void;
   +getSimpleMemoryInfo: () => NativeMemoryInfo;
 }
 

diff --git a/Libraries/WebPerformance/NativePerformanceObserver.cpp b/Libraries/WebPerformance/NativePerformanceObserver.cpp
@@ -78,4 +78,13 @@ void NativePerformanceObserver::setDurationThreshold(
       static_cast<PerformanceEntryType>(entryType), durationThreshold);
 }
 
+void NativePerformanceObserver::clearEntries(
+    jsi::Runtime &rt,
+    int32_t entryType,
+    std::optional<std::string> entryName) {
+  PerformanceEntryReporter::getInstance().clearEntries(
+      static_cast<PerformanceEntryType>(entryType),
+      entryName ? entryName->c_str() : nullptr);
+}
+
 } // namespace facebook::react
diff --git a/Libraries/WebPerformance/NativePerformanceObserver.h b/Libraries/WebPerformance/NativePerformanceObserver.h
@@ -79,6 +79,11 @@ class NativePerformanceObserver
       int32_t entryType,
       double durationThreshold);
 
+  void clearEntries(
+      jsi::Runtime &rt,
+      int32_t entryType,
+      std::optional<std::string> entryName);
+
  private:
 };
 

diff --git a/Libraries/WebPerformance/NativePerformanceObserver.js b/Libraries/WebPerformance/NativePerformanceObserver.js
@@ -41,6 +41,10 @@ export interface Spec extends TurboModule {
     entryType: RawPerformanceEntryType,
     durationThreshold: number,
   ) => void;
+  +clearEntries: (
+    entryType: RawPerformanceEntryType,
+    entryName?: string,
+  ) => void;
 }
 
 export default (TurboModuleRegistry.get<Spec>(

diff --git a/Libraries/WebPerformance/Performance.js b/Libraries/WebPerformance/Performance.js
@@ -16,7 +16,10 @@ import warnOnce from '../Utilities/warnOnce';
 import EventCounts from './EventCounts';
 import MemoryInfo from './MemoryInfo';
 import NativePerformance from './NativePerformance';
+import NativePerformanceObserver from './NativePerformanceObserver';
 import {PerformanceEntry} from './PerformanceEntry';
+import {warnNoNativePerformanceObserver} from './PerformanceObserver';
+import {RawPerformanceEntryTypeValues} from './RawPerformanceEntry';
 
 type DetailType = mixed;
 
@@ -135,12 +138,15 @@ export default class Performance {
   }
 
   clearMarks(markName?: string): void {
-    if (!NativePerformance?.clearMarks) {
-      warnNoNativePerformance();
+    if (!NativePerformanceObserver?.clearEntries) {
+      warnNoNativePerformanceObserver();
       return;
     }
 
-    NativePerformance.clearMarks(markName);
+    NativePerformanceObserver?.clearEntries(
+      RawPerformanceEntryTypeValues.MARK,
+      markName,
+    );
   }
 
   measure(
@@ -213,12 +219,15 @@ export default class Performance {
   }
 
   clearMeasures(measureName?: string): void {
-    if (!NativePerformance?.clearMeasures) {
-      warnNoNativePerformance();
+    if (!NativePerformanceObserver?.clearEntries) {
+      warnNoNativePerformanceObserver();
       return;
     }
 
-    NativePerformance.clearMeasures(measureName);
+    NativePerformanceObserver?.clearEntries(
+      RawPerformanceEntryTypeValues.MEASURE,
+      measureName,
+    );
   }
 
   /**

diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.cpp b/Libraries/WebPerformance/PerformanceEntryReporter.cpp
@@ -121,21 +121,27 @@ void PerformanceEntryReporter::mark(
        std::nullopt});
 }
 
-void PerformanceEntryReporter::clearMarks(
-    const std::optional<std::string> &markName) {
-  if (markName) {
-    PerformanceMark mark{{*markName, 0}};
+void PerformanceEntryReporter::clearEntries(
+    PerformanceEntryType entryType,
+    const char *entryName) {
+  if (entryName != nullptr && entryType == PerformanceEntryType::MARK) {
+    // remove a named mark from the mark/measure registry
+    PerformanceMark mark{{entryName, 0}};
     marksRegistry_.erase(&mark);
-    clearEntries([&markName](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MARK) &&
-          entry.name == markName;
-    });
-  } else {
-    marksRegistry_.clear();
-    clearEntries([](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MARK);
-    });
   }
+
+  int lastPos = entries_.size() - 1;
+  int pos = lastPos;
+  while (pos >= 0) {
+    const RawPerformanceEntry &entry = entries_[pos];
+    if (entry.entryType == static_cast<int32_t>(entryType) &&
+        (entryName == nullptr || entry.name == entryName)) {
+      entries_[pos] = entries_[lastPos];
+      lastPos--;
+    }
+    pos--;
+  }
+  entries_.resize(lastPos + 1);
 }
 
 void PerformanceEntryReporter::measure(
@@ -158,22 +164,6 @@ void PerformanceEntryReporter::measure(
        std::nullopt});
 }
 
-void PerformanceEntryReporter::clearMeasures(
-    const std::optional<std::string> &measureName) {
-  if (measureName) {
-    clearEntries([&measureName](const RawPerformanceEntry &entry) {
-      return entry.entryType ==
-          static_cast<int>(PerformanceEntryType::MEASURE) &&
-          entry.name == measureName;
-    });
-  } else {
-    marksRegistry_.clear();
-    clearEntries([](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MEASURE);
-    });
-  }
-}
-
 double PerformanceEntryReporter::getMarkTime(
     const std::string &markName) const {
   PerformanceMark mark{{std::move(markName), 0}};
@@ -202,20 +192,6 @@ void PerformanceEntryReporter::event(
        interactionId});
 }
 
-void PerformanceEntryReporter::clearEntries(
-    std::function<bool(const RawPerformanceEntry &)> predicate) {
-  int lastPos = entries_.size() - 1;
-  int pos = lastPos;
-  while (pos >= 0) {
-    if (predicate(entries_[pos])) {
-      entries_[pos] = entries_[lastPos];
-      lastPos--;
-    }
-    pos--;
-  }
-  entries_.resize(lastPos + 1);
-}
-
 void PerformanceEntryReporter::scheduleFlushBuffer() {
   if (callback_) {
     callback_->callWithPriority(SchedulerPriority::IdlePriority);

diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.h b/Libraries/WebPerformance/PerformanceEntryReporter.h
@@ -84,7 +84,6 @@ class PerformanceEntryReporter : public EventLogger {
   }
 
   void mark(const std::string &name, double startTime, double duration);
-  void clearMarks(const std::optional<std::string> &markName);
 
   void measure(
       const std::string &name,
@@ -93,7 +92,10 @@ class PerformanceEntryReporter : public EventLogger {
       const std::optional<double> &duration,
       const std::optional<std::string> &startMark,
       const std::optional<std::string> &endMark);
-  void clearMeasures(const std::optional<std::string> &measureName);
+
+  void clearEntries(
+      PerformanceEntryType entryType,
+      const char *entryName = nullptr);
 
   void event(
       std::string name,
@@ -115,7 +117,6 @@ class PerformanceEntryReporter : public EventLogger {
   PerformanceEntryReporter() {}
 
   double getMarkTime(const std::string &markName) const;
-  void clearEntries(std::function<bool(const RawPerformanceEntry &)> predicate);
   void scheduleFlushBuffer();
 
   bool isReportingEvents() const {

diff --git a/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js b/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js
@@ -77,6 +77,14 @@ const NativePerformanceObserverMock: NativePerformanceObserver = {
   ) => {
     durationThresholds.set(entryType, durationThreshold);
   },
+
+  clearEntries: (entryType: RawPerformanceEntryType, entryName?: string) => {
+    entries = entries.filter(
+      e =>
+        e.entryType === entryType &&
+        (entryName == null || e.name === entryName),
+    );
+  },
 };
 
 export default NativePerformanceObserverMock;