From 44755726ed3dd59a21cfe10fb996fe720357eee8 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Wed, 14 Jun 2023 11:20:10 -0700 Subject: [PATCH] Performance.measure: handle the case with a single startMark argument correctly (#37888) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37888 # Changelog: [Internal] - There was one particular permutation of input arguments to `Performance.measure` that wasn't handled correctly on the native side, namely when there is only the start mark argument present, but not the end time/mark, e.g.: ``` Performance.measure('myMeasure', 'someStartMark'); ``` In this case, [according to the standard](https://developer.mozilla.org/en-US/docs/Web/API/Performance/measure), the end time should be taken as the current one: > The end timestamp is one of: > ... > -the value returned by Performance.now(), if no end mark is specified or can be determined from other values. It was taken as 0 instead, making the total duration negative and consequently getting it filtered out by the default `durationThreshold` of 0. I've added a corresponding missing clause in the native unit tests. This also required a slight extension to the `PerformanceObserver` API to allow for mocking the current timestamp provider. Reviewed By: rubennorte Differential Revision: D46728261 fbshipit-source-id: bd904d9c93707fa04c1a0ddb30802691e253c106 --- .../PerformanceEntryReporter.cpp | 18 ++++++++++++++---- .../WebPerformance/PerformanceEntryReporter.h | 8 ++++++++ .../__tests__/PerformanceEntryReporterTest.cpp | 14 ++++++++++++-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp index e223156ac94..3d8d3aba94d 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; @@ -29,11 +25,17 @@ PerformanceEntryReporter::PerformanceEntryReporter() { // sure that marks can be referenced by measures getBuffer(PerformanceEntryType::MARK).hasNameLookup = true; } + void PerformanceEntryReporter::setReportingCallback( std::optional> callback) { callback_ = callback; } +double PerformanceEntryReporter::getCurrentTimeStamp() const { + return timeStampProvider_ != nullptr ? timeStampProvider_() + : JSExecutor::performanceNow(); +} + void PerformanceEntryReporter::startReporting(PerformanceEntryType entryType) { auto &buffer = getBuffer(entryType); buffer.isReporting = true; @@ -219,7 +221,15 @@ void PerformanceEntryReporter::measure( const std::optional &endMark) { double startTimeVal = startMark ? getMarkTime(*startMark) : startTime; double endTimeVal = endMark ? getMarkTime(*endMark) : endTime; + + if (!endMark && endTime < startTimeVal) { + // The end time is not specified, take the current time, according to the + // standard + endTimeVal = getCurrentTimeStamp(); + } + double durationVal = duration ? *duration : endTimeVal - startTimeVal; + logEntry( {name, static_cast(PerformanceEntryType::MEASURE), diff --git a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h index 8ea7c820e01..a68df9247a4 100644 --- a/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h +++ b/packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h @@ -147,6 +147,10 @@ class PerformanceEntryReporter : public EventLogger { return eventCounts_; } + void setTimeStampProvider(std::function provider) { + timeStampProvider_ = provider; + } + private: std::optional> callback_; @@ -171,6 +175,8 @@ class PerformanceEntryReporter : public EventLogger { std::unordered_map eventsInFlight_; std::mutex eventsInFlightMutex_; + std::function timeStampProvider_ = nullptr; + static EventTag sCurrentEventTag_; PerformanceEntryReporter(); @@ -182,6 +188,8 @@ class PerformanceEntryReporter : public EventLogger { PerformanceEntryType entryType, const char *entryName, std::vector &res) const; + + double getCurrentTimeStamp() const; }; } // namespace facebook::react diff --git a/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp b/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp index c5895021ee6..e7e8952a89d 100644 --- a/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp +++ b/packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp @@ -146,8 +146,11 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { reporter.measure("measure3", 0.0, 0.0, 5.0, "mark1"); reporter.measure("measure4", 1.5, 0.0, std::nullopt, std::nullopt, "mark2"); + reporter.setTimeStampProvider([]() { return 3.5; }); + reporter.measure("measure5", 0.0, 0.0, std::nullopt, "mark2"); + reporter.mark("mark3", 2.0); - reporter.measure("measure5", 2.0, 2.0); + reporter.measure("measure6", 2.0, 2.0); reporter.mark("mark4", 2.0); auto res = reporter.popPendingEntries(); @@ -226,13 +229,20 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) { std::nullopt, std::nullopt, std::nullopt}, - {"measure5", + {"measure6", static_cast(PerformanceEntryType::MEASURE), 2.0, 0.0, std::nullopt, std::nullopt, std::nullopt}, + {"measure5", + static_cast(PerformanceEntryType::MEASURE), + 2.0, + 1.5, + std::nullopt, + std::nullopt, + std::nullopt}, }; ASSERT_EQ(expected, entries);