From 9e0d198727d1e3018666a705797b00a7e9edfbf3 Mon Sep 17 00:00:00 2001 From: Oleksandr Melnykov Date: Thu, 20 Apr 2023 15:36:16 -0700 Subject: [PATCH] Back out "Clean up uneeded "duration" parameter from the Performance.mark API" Summary: Changelog: [Internal] Original commit changeset: ce3d14940124 Original Phabricator Diff: D45141978 Reviewed By: GijsWeterings Differential Revision: D45161958 fbshipit-source-id: 22d79e8e8d0670fd220391f7e5ea92ca257ff982 --- .../WebPerformance/NativePerformance.cpp | 5 +- .../WebPerformance/NativePerformance.h | 3 +- .../WebPerformance/NativePerformance.js | 2 +- .../Libraries/WebPerformance/Performance.js | 2 +- .../PerformanceEntryReporter.cpp | 17 +++--- .../WebPerformance/PerformanceEntryReporter.h | 4 +- .../__mocks__/NativePerformance.js | 4 +- .../PerformanceEntryReporterTest.cpp | 56 +++++++++---------- 8 files changed, 45 insertions(+), 48 deletions(-) diff --git a/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp b/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp index e9ccdf332b6..d08810da47c 100644 --- a/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp +++ b/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp @@ -28,8 +28,9 @@ NativePerformance::NativePerformance(std::shared_ptr jsInvoker) void NativePerformance::mark( jsi::Runtime &rt, std::string name, - double startTime) { - PerformanceEntryReporter::getInstance().mark(name, startTime); + double startTime, + double duration) { + PerformanceEntryReporter::getInstance().mark(name, startTime, duration); } void NativePerformance::measure( diff --git a/packages/react-native/Libraries/WebPerformance/NativePerformance.h b/packages/react-native/Libraries/WebPerformance/NativePerformance.h index 1726bb663f3..4b334d7339b 100644 --- a/packages/react-native/Libraries/WebPerformance/NativePerformance.h +++ b/packages/react-native/Libraries/WebPerformance/NativePerformance.h @@ -41,7 +41,8 @@ class NativePerformance : public NativePerformanceCxxSpec, public: NativePerformance(std::shared_ptr jsInvoker); - void mark(jsi::Runtime &rt, std::string name, double startTime); + void + mark(jsi::Runtime &rt, std::string name, double startTime, double duration); void measure( jsi::Runtime &rt, diff --git a/packages/react-native/Libraries/WebPerformance/NativePerformance.js b/packages/react-native/Libraries/WebPerformance/NativePerformance.js index 092b4ff6875..9a34b231299 100644 --- a/packages/react-native/Libraries/WebPerformance/NativePerformance.js +++ b/packages/react-native/Libraries/WebPerformance/NativePerformance.js @@ -22,7 +22,7 @@ export type ReactNativeStartupTiming = {| |}; export interface Spec extends TurboModule { - +mark: (name: string, startTime: number) => void; + +mark: (name: string, startTime: number, duration: number) => void; +measure: ( name: string, startTime: number, diff --git a/packages/react-native/Libraries/WebPerformance/Performance.js b/packages/react-native/Libraries/WebPerformance/Performance.js index 5b6fc03a183..8e2ce32bd82 100644 --- a/packages/react-native/Libraries/WebPerformance/Performance.js +++ b/packages/react-native/Libraries/WebPerformance/Performance.js @@ -157,7 +157,7 @@ export default class Performance { const mark = new PerformanceMark(markName, markOptions); if (NativePerformance?.mark) { - NativePerformance.mark(markName, mark.startTime); + NativePerformance.mark(markName, mark.startTime, mark.duration); } else { warnNoNativePerformance(); } diff --git a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp index 8c82a5c2a9d..024178f344a 100644 --- a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp +++ b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp @@ -15,10 +15,6 @@ namespace facebook::react { EventTag PerformanceEntryReporter::sCurrentEventTag_{0}; -static inline double getCurrentTimeStamp() { - return JSExecutor::performanceNow(); -} - PerformanceEntryReporter &PerformanceEntryReporter::getInstance() { static PerformanceEntryReporter instance; return instance; @@ -138,12 +134,13 @@ void PerformanceEntryReporter::logEntry(const RawPerformanceEntry &entry) { void PerformanceEntryReporter::mark( const std::string &name, - const std::optional &startTime) { + double startTime, + double duration) { logEntry(RawPerformanceEntry{ name, static_cast(PerformanceEntryType::MARK), - startTime ? *startTime : getCurrentTimeStamp(), - 0.0, + startTime, + duration, std::nullopt, std::nullopt, std::nullopt}); @@ -355,7 +352,7 @@ EventTag PerformanceEntryReporter::onEventStart(const char *name) { sCurrentEventTag_ = 1; } - auto timeStamp = getCurrentTimeStamp(); + auto timeStamp = JSExecutor::performanceNow(); { std::lock_guard lock(eventsInFlightMutex_); eventsInFlight_.emplace(std::make_pair( @@ -368,7 +365,7 @@ void PerformanceEntryReporter::onEventDispatch(EventTag tag) { if (!isReporting(PerformanceEntryType::EVENT) || tag == 0) { return; } - auto timeStamp = getCurrentTimeStamp(); + auto timeStamp = JSExecutor::performanceNow(); { std::lock_guard lock(eventsInFlightMutex_); auto it = eventsInFlight_.find(tag); @@ -382,7 +379,7 @@ void PerformanceEntryReporter::onEventEnd(EventTag tag) { if (!isReporting(PerformanceEntryType::EVENT) || tag == 0) { return; } - auto timeStamp = getCurrentTimeStamp(); + auto timeStamp = JSExecutor::performanceNow(); { std::lock_guard lock(eventsInFlightMutex_); auto it = eventsInFlight_.find(tag); diff --git a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h index 8ea7c820e01..4ce7794078f 100644 --- a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h +++ b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h @@ -111,9 +111,7 @@ class PerformanceEntryReporter : public EventLogger { return droppedEntryCount_; } - void mark( - const std::string &name, - const std::optional &startTime = std::nullopt); + void mark(const std::string &name, double startTime, double duration); void measure( const std::string &name, diff --git a/packages/react-native/Libraries/WebPerformance/__mocks__/NativePerformance.js b/packages/react-native/Libraries/WebPerformance/__mocks__/NativePerformance.js index b3335e0c774..6b11a27a20a 100644 --- a/packages/react-native/Libraries/WebPerformance/__mocks__/NativePerformance.js +++ b/packages/react-native/Libraries/WebPerformance/__mocks__/NativePerformance.js @@ -20,12 +20,12 @@ import {RawPerformanceEntryTypeValues} from '../RawPerformanceEntry'; const marks: Map = new Map(); const NativePerformanceMock: NativePerformance = { - mark: (name: string, startTime: number): void => { + mark: (name: string, startTime: number, duration: number): void => { NativePerformanceObserver?.logRawEntry({ name, entryType: RawPerformanceEntryTypeValues.MARK, startTime, - duration: 0, + duration, }); marks.set(name, startTime); }, diff --git a/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp b/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp index fb2dabc0d96..9a6842eec2c 100644 --- a/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp +++ b/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp @@ -54,9 +54,9 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestStopReporting) { reporter.startReporting(PerformanceEntryType::MARK); - reporter.mark("mark0", 0.0); - reporter.mark("mark1", 0.0); - reporter.mark("mark2", 0.0); + reporter.mark("mark0", 0.0, 0.0); + reporter.mark("mark1", 0.0, 0.0); + reporter.mark("mark2", 0.0, 0.0); reporter.measure("measure0", 0.0, 0.0); auto res = reporter.popPendingEntries(); @@ -73,7 +73,7 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestStopReporting) { reporter.stopReporting(PerformanceEntryType::MARK); reporter.startReporting(PerformanceEntryType::MEASURE); - reporter.mark("mark3"); + reporter.mark("mark3", 0.0, 0.0); reporter.measure("measure1", 0.0, 0.0); res = reporter.popPendingEntries(); @@ -91,9 +91,9 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMarks) { reporter.startReporting(PerformanceEntryType::MARK); - reporter.mark("mark0", 0.0); - reporter.mark("mark1", 1.0); - reporter.mark("mark2", 2.0); + reporter.mark("mark0", 0.0, 1.0); + reporter.mark("mark1", 1.0, 3.0); + reporter.mark("mark2", 2.0, 4.0); auto res = reporter.popPendingEntries(); const auto &entries = res.entries; @@ -105,21 +105,21 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMarks) { {"mark0", static_cast(PerformanceEntryType::MARK), 0.0, - 0.0, + 1.0, std::nullopt, std::nullopt, std::nullopt}, {"mark1", static_cast(PerformanceEntryType::MARK), 1.0, - 0.0, + 3.0, std::nullopt, std::nullopt, std::nullopt}, {"mark2", static_cast(PerformanceEntryType::MARK), 2.0, - 0.0, + 4.0, std::nullopt, std::nullopt, std::nullopt}}; @@ -136,9 +136,9 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { reporter.startReporting(PerformanceEntryType::MARK); reporter.startReporting(PerformanceEntryType::MEASURE); - reporter.mark("mark0", 0.0); - reporter.mark("mark1", 1.0); - reporter.mark("mark2", 2.0); + reporter.mark("mark0", 0.0, 1.0); + reporter.mark("mark1", 1.0, 3.0); + reporter.mark("mark2", 2.0, 4.0); reporter.measure("measure0", 0.0, 2.0); reporter.measure("measure1", 0.0, 2.0, 4.0); @@ -155,7 +155,7 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { {"mark0", static_cast(PerformanceEntryType::MARK), 0.0, - 0.0, + 1.0, std::nullopt, std::nullopt, std::nullopt}, @@ -173,13 +173,6 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { std::nullopt, std::nullopt, std::nullopt}, - {"mark1", - static_cast(PerformanceEntryType::MARK), - 1.0, - 0.0, - std::nullopt, - std::nullopt, - std::nullopt}, {"measure2", static_cast(PerformanceEntryType::MEASURE), 1.0, @@ -187,6 +180,13 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { std::nullopt, std::nullopt, std::nullopt}, + {"mark1", + static_cast(PerformanceEntryType::MARK), + 1.0, + 3.0, + std::nullopt, + std::nullopt, + std::nullopt}, {"measure3", static_cast(PerformanceEntryType::MEASURE), 1.0, @@ -204,7 +204,7 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { {"mark2", static_cast(PerformanceEntryType::MARK), 2.0, - 0.0, + 4.0, std::nullopt, std::nullopt, std::nullopt}}; @@ -249,9 +249,9 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestGetEntries) { reporter.startReporting(PerformanceEntryType::MARK); reporter.startReporting(PerformanceEntryType::MEASURE); - reporter.mark("common_name", 0.0); - reporter.mark("mark1", 1.0); - reporter.mark("mark2", 2.0); + reporter.mark("common_name", 0.0, 1.0); + reporter.mark("mark1", 1.0, 3.0); + reporter.mark("mark2", 2.0, 4.0); reporter.measure("common_name", 0.0, 2.0); reporter.measure("measure1", 0.0, 2.0, 4.0); @@ -296,9 +296,9 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestClearEntries) { reporter.startReporting(PerformanceEntryType::MARK); reporter.startReporting(PerformanceEntryType::MEASURE); - reporter.mark("common_name", 0.0); - reporter.mark("mark1", 1.0); - reporter.mark("mark2", 2.0); + reporter.mark("common_name", 0.0, 1.0); + reporter.mark("mark1", 1.0, 3.0); + reporter.mark("mark2", 2.0, 4.0); reporter.measure("common_name", 0.0, 2.0); reporter.measure("measure1", 0.0, 2.0, 4.0);