From 2b95380e85a724705e5625030607ada7a5faa1f5 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Tue, 14 Feb 2023 15:54:53 -0800 Subject: [PATCH] Use proper enums to communicate between JS and C++ in NativePerformanceObserver Summary: [Changelog][Internal] After D42884147 (https://github.com/facebook/react-native/commit/ceb1d0dea694739f357d86296b94f5834e5ee7f7) (https://github.com/facebook/react-native/pull/36030) had landed, we now can use enums as data types when bridging between JS and C++ TurboModules. This diff takes advantage of this and converts the API to use the proper enums instead of int types that it was forced to use earlier. NOTE: This also relies on additional change to codegen, D43292254, which is required in order for enums to work also as part of data structures. Differential Revision: D43292282 fbshipit-source-id: fd271704ab60aec2b3ae5b260e76665ad50e206e --- .../NativePerformanceObserver.cpp | 20 ++++++------- .../NativePerformanceObserver.h | 13 +++++---- .../NativePerformanceObserver.js | 8 ++++- .../PerformanceEntryReporter.cpp | 26 ++++++++--------- .../WebPerformance/PerformanceEntryReporter.h | 8 ----- .../WebPerformance/RawPerformanceEntry.js | 29 +++++++------------ .../__mocks__/NativePerformanceObserver.js | 5 ++-- 7 files changed, 49 insertions(+), 60 deletions(-) diff --git a/Libraries/WebPerformance/NativePerformanceObserver.cpp b/Libraries/WebPerformance/NativePerformanceObserver.cpp index 98dfb05af1bf72..93f9bffd26b9e5 100644 --- a/Libraries/WebPerformance/NativePerformanceObserver.cpp +++ b/Libraries/WebPerformance/NativePerformanceObserver.cpp @@ -22,18 +22,26 @@ NativePerformanceObserver::~NativePerformanceObserver() { void NativePerformanceObserver::startReporting( jsi::Runtime &rt, - int32_t entryType) { + PerformanceEntryType entryType) { PerformanceEntryReporter::getInstance().startReporting( static_cast(entryType)); } void NativePerformanceObserver::stopReporting( jsi::Runtime &rt, - int32_t entryType) { + PerformanceEntryType entryType) { PerformanceEntryReporter::getInstance().stopReporting( static_cast(entryType)); } +void NativePerformanceObserver::setDurationThreshold( + jsi::Runtime &rt, + PerformanceEntryType entryType, + double durationThreshold) { + PerformanceEntryReporter::getInstance().setDurationThreshold( + static_cast(entryType), durationThreshold); +} + GetPendingEntriesResult NativePerformanceObserver::popPendingEntries( jsi::Runtime &rt) { return PerformanceEntryReporter::getInstance().popPendingEntries(); @@ -51,14 +59,6 @@ void NativePerformanceObserver::logRawEntry( PerformanceEntryReporter::getInstance().logEntry(entry); } -void NativePerformanceObserver::setDurationThreshold( - jsi::Runtime &rt, - int32_t entryType, - double durationThreshold) { - PerformanceEntryReporter::getInstance().setDurationThreshold( - static_cast(entryType), durationThreshold); -} - std::vector> NativePerformanceObserver::getEventCounts(jsi::Runtime &rt) { const auto &eventCounts = diff --git a/Libraries/WebPerformance/NativePerformanceObserver.h b/Libraries/WebPerformance/NativePerformanceObserver.h index 196ea4c53bf1fb..d0866a88d5b822 100644 --- a/Libraries/WebPerformance/NativePerformanceObserver.h +++ b/Libraries/WebPerformance/NativePerformanceObserver.h @@ -18,9 +18,12 @@ class PerformanceEntryReporter; #pragma mark - Structs +using PerformanceEntryType = + NativePerformanceObserverCxxRawPerformanceEntryType; + using RawPerformanceEntry = NativePerformanceObserverCxxBaseRawPerformanceEntry< std::string, - int32_t, + PerformanceEntryType, double, double, // For "event" entries only: @@ -32,7 +35,7 @@ template <> struct Bridging : NativePerformanceObserverCxxBaseRawPerformanceEntryBridging< std::string, - int32_t, + PerformanceEntryType, double, double, std::optional, @@ -59,9 +62,9 @@ class NativePerformanceObserver NativePerformanceObserver(std::shared_ptr jsInvoker); ~NativePerformanceObserver(); - void startReporting(jsi::Runtime &rt, int32_t entryType); + void startReporting(jsi::Runtime &rt, PerformanceEntryType entryType); - void stopReporting(jsi::Runtime &rt, int32_t entryType); + void stopReporting(jsi::Runtime &rt, PerformanceEntryType entryType); GetPendingEntriesResult popPendingEntries(jsi::Runtime &rt); @@ -73,7 +76,7 @@ class NativePerformanceObserver void setDurationThreshold( jsi::Runtime &rt, - int32_t entryType, + PerformanceEntryType entryType, double durationThreshold); std::vector> getEventCounts( diff --git a/Libraries/WebPerformance/NativePerformanceObserver.js b/Libraries/WebPerformance/NativePerformanceObserver.js index 5724d7cedfd233..09faf3e81aaddf 100644 --- a/Libraries/WebPerformance/NativePerformanceObserver.js +++ b/Libraries/WebPerformance/NativePerformanceObserver.js @@ -12,7 +12,13 @@ import type {TurboModule} from '../TurboModule/RCTExport'; import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry'; -export type RawPerformanceEntryType = number; +export enum RawPerformanceEntryType { + UNDEFINED = 0, + MARK = 1, + MEASURE = 2, + EVENT = 3, + _COUNT = 4, +} export type RawPerformanceEntry = {| name: string, diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.cpp b/Libraries/WebPerformance/PerformanceEntryReporter.cpp index a741857de946c9..dde81a4f937b00 100644 --- a/Libraries/WebPerformance/PerformanceEntryReporter.cpp +++ b/Libraries/WebPerformance/PerformanceEntryReporter.cpp @@ -31,7 +31,7 @@ void PerformanceEntryReporter::setReportingCallback( } void PerformanceEntryReporter::startReporting(PerformanceEntryType entryType) { - int entryTypeIdx = static_cast(entryType); + size_t entryTypeIdx = static_cast(entryType); reportingType_[entryTypeIdx] = true; durationThreshold_[entryTypeIdx] = DEFAULT_DURATION_THRESHOLD; } @@ -39,11 +39,11 @@ void PerformanceEntryReporter::startReporting(PerformanceEntryType entryType) { void PerformanceEntryReporter::setDurationThreshold( PerformanceEntryType entryType, double durationThreshold) { - durationThreshold_[static_cast(entryType)] = durationThreshold; + durationThreshold_[static_cast(entryType)] = durationThreshold; } void PerformanceEntryReporter::stopReporting(PerformanceEntryType entryType) { - reportingType_[static_cast(entryType)] = false; + reportingType_[static_cast(entryType)] = false; } GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() { @@ -56,12 +56,11 @@ GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() { } void PerformanceEntryReporter::logEntry(const RawPerformanceEntry &entry) { - const auto entryType = static_cast(entry.entryType); - if (entryType == PerformanceEntryType::EVENT) { + if (entry.entryType == PerformanceEntryType::EVENT) { eventCounts_[entry.name]++; } - if (!isReportingType(entryType)) { + if (!isReportingType(entry.entryType)) { return; } @@ -113,7 +112,7 @@ void PerformanceEntryReporter::mark( logEntry( {name, - static_cast(PerformanceEntryType::MARK), + PerformanceEntryType::MARK, startTime, duration, std::nullopt, @@ -127,13 +126,13 @@ void PerformanceEntryReporter::clearMarks( PerformanceMark mark{{*markName, 0}}; marksRegistry_.erase(&mark); clearEntries([&markName](const RawPerformanceEntry &entry) { - return entry.entryType == static_cast(PerformanceEntryType::MARK) && + return entry.entryType == PerformanceEntryType::MARK && entry.name == markName; }); } else { marksRegistry_.clear(); clearEntries([](const RawPerformanceEntry &entry) { - return entry.entryType == static_cast(PerformanceEntryType::MARK); + return entry.entryType == PerformanceEntryType::MARK; }); } } @@ -150,7 +149,7 @@ void PerformanceEntryReporter::measure( double durationVal = duration ? *duration : endTimeVal - startTimeVal; logEntry( {name, - static_cast(PerformanceEntryType::MEASURE), + PerformanceEntryType::MEASURE, startTimeVal, durationVal, std::nullopt, @@ -162,14 +161,13 @@ void PerformanceEntryReporter::clearMeasures( const std::optional &measureName) { if (measureName) { clearEntries([&measureName](const RawPerformanceEntry &entry) { - return entry.entryType == - static_cast(PerformanceEntryType::MEASURE) && + return entry.entryType == PerformanceEntryType::MEASURE && entry.name == measureName; }); } else { marksRegistry_.clear(); clearEntries([](const RawPerformanceEntry &entry) { - return entry.entryType == static_cast(PerformanceEntryType::MEASURE); + return entry.entryType == PerformanceEntryType::MEASURE; }); } } @@ -194,7 +192,7 @@ void PerformanceEntryReporter::event( uint32_t interactionId) { logEntry( {std::move(name), - static_cast(PerformanceEntryType::EVENT), + PerformanceEntryType::EVENT, startTime, duration, processingStart, diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.h b/Libraries/WebPerformance/PerformanceEntryReporter.h index 417cf9e3172c2b..ed5f47cbddbd1a 100644 --- a/Libraries/WebPerformance/PerformanceEntryReporter.h +++ b/Libraries/WebPerformance/PerformanceEntryReporter.h @@ -46,14 +46,6 @@ constexpr size_t MARKS_BUFFER_SIZE = 1024; constexpr double DEFAULT_DURATION_THRESHOLD = 0.0; -enum class PerformanceEntryType { - UNDEFINED = 0, - MARK = 1, - MEASURE = 2, - EVENT = 3, - _COUNT = 4, -}; - class PerformanceEntryReporter : public EventLogger { public: PerformanceEntryReporter(PerformanceEntryReporter const &) = delete; diff --git a/Libraries/WebPerformance/RawPerformanceEntry.js b/Libraries/WebPerformance/RawPerformanceEntry.js index 1b1bdbb95a8205..d52d73fce8580c 100644 --- a/Libraries/WebPerformance/RawPerformanceEntry.js +++ b/Libraries/WebPerformance/RawPerformanceEntry.js @@ -8,26 +8,17 @@ * @flow strict */ -import type { - RawPerformanceEntry, - RawPerformanceEntryType, -} from './NativePerformanceObserver'; +import type {RawPerformanceEntry} from './NativePerformanceObserver'; import type {PerformanceEntryType} from './PerformanceEntry'; +import {RawPerformanceEntryType} from './NativePerformanceObserver'; import {PerformanceEntry} from './PerformanceEntry'; import {PerformanceEventTiming} from './PerformanceEventTiming'; -export const RawPerformanceEntryTypeValues = { - UNDEFINED: 0, - MARK: 1, - MEASURE: 2, - EVENT: 3, -}; - export function rawToPerformanceEntry( entry: RawPerformanceEntry, ): PerformanceEntry { - if (entry.entryType === RawPerformanceEntryTypeValues.EVENT) { + if (entry.entryType === RawPerformanceEntryType.EVENT) { return new PerformanceEventTiming({ name: entry.name, startTime: entry.startTime, @@ -50,15 +41,15 @@ export function rawToPerformanceEntryType( type: RawPerformanceEntryType, ): PerformanceEntryType { switch (type) { - case RawPerformanceEntryTypeValues.MARK: + case RawPerformanceEntryType.MARK: return 'mark'; - case RawPerformanceEntryTypeValues.MEASURE: + case RawPerformanceEntryType.MEASURE: return 'measure'; - case RawPerformanceEntryTypeValues.EVENT: + case RawPerformanceEntryType.EVENT: return 'event'; default: throw new TypeError( - `rawToPerformanceEntryType: unexpected performance entry type received: ${type}`, + 'rawToPerformanceEntryType: unexpected RawPerformanceEntryType received', ); } } @@ -68,11 +59,11 @@ export function performanceEntryTypeToRaw( ): RawPerformanceEntryType { switch (type) { case 'mark': - return RawPerformanceEntryTypeValues.MARK; + return RawPerformanceEntryType.MARK; case 'measure': - return RawPerformanceEntryTypeValues.MEASURE; + return RawPerformanceEntryType.MEASURE; case 'event': - return RawPerformanceEntryTypeValues.EVENT; + return RawPerformanceEntryType.EVENT; default: // Verify exhaustive check with Flow (type: empty); diff --git a/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js b/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js index 597b32603c4be4..42e93cbeca8aa2 100644 --- a/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js +++ b/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js @@ -11,11 +11,10 @@ import type { GetPendingEntriesResult, RawPerformanceEntry, - RawPerformanceEntryType, Spec as NativePerformanceObserver, } from '../NativePerformanceObserver'; -import {RawPerformanceEntryTypeValues} from '../RawPerformanceEntry'; +import {RawPerformanceEntryType} from '../NativePerformanceObserver'; const reportingType: Set = new Set(); const durationThresholds: Map = new Map(); @@ -58,7 +57,7 @@ export const NativePerformanceObserverMock: NativePerformanceObserver = { entries.push(entry); onPerformanceEntryCallback?.(); } - if (entry.entryType === RawPerformanceEntryTypeValues.EVENT) { + if (entry.entryType === RawPerformanceEntryType.EVENT) { eventCounts.set(entry.name, (eventCounts.get(entry.name) ?? 0) + 1); } },