Skip to content

Commit 4edc33e

Browse files
rubennortefacebook-github-bot
rubennorte
authored and
facebook-github-bot
committed
Refactor implementation of performance.mark and performance.measure (#52586)
Summary: Pull Request resolved: #52586 Changelog: [internal] This fixes a "bug" (or spec-compliance issue) in the `performance.measure` method where `end` and `duration` couldn't be used together (only `start` and `duration`, and `start` and `end` could be used). This also refactors the API to be "JS-first", preparing the native module methods to support passing instances of entries to fix other issues. Verified performance impact: `mark` is slightly regressed (~7% slower) due to an additional JSI call to get the default start time and `measure` is significantly optimized (25/30% faster) due to simplified parameter handling in JSI calls. * Before | (index) | Task name | Latency average (ns) | Latency median (ns) | Throughput average (ops/s) | Throughput median (ops/s) | Samples | | ------- | --------------------------------------------------------- | -------------------- | ------------------- | -------------------------- | ------------------------- | ------- | | 0 | 'mark (default)' | '1621.03 ± 1.17%' | '1562.00' | '636986 ± 0.01%' | '640205' | 616890 | | 1 | 'mark (with custom startTime)' | '1756.88 ± 1.15%' | '1693.00' | '586826 ± 0.01%' | '590667' | 569190 | | 2 | 'measure (default)' | '2424.66 ± 1.35%' | '2333.00' | '426122 ± 0.02%' | '428633' | 412429 | | 3 | 'measure (with start and end timestamps)' | '2679.96 ± 1.23%' | '2574.00' | '385266 ± 0.02%' | '388500' | 373140 | | 4 | 'measure (with mark names)' | '2713.49 ± 0.50%' | '2644.00' | '375383 ± 0.02%' | '378215' | 368530 | | 5 | 'clearMarks' | '691.13 ± 0.07%' | '681.00' | '1467016 ± 0.01%' | '1468429' | 1446900 | | 6 | 'clearMeasures' | '706.00 ± 0.05%' | '691.00' | '1431489 ± 0.01%' | '1447178' | 1416435 | | 7 | 'mark + clearMarks' | '2083.21 ± 1.14%' | '2003.00' | '497974 ± 0.01%' | '499251' | 480028 | | 8 | 'measure + clearMeasures (with start and end timestamps)' | '3085.14 ± 0.88%' | '2974.00' | '334337 ± 0.02%' | '336247' | 324135 | | 9 | 'measure + clearMeasures (with mark names)' | '2949.45 ± 0.62%' | '2884.00' | '345335 ± 0.02%' | '346741' | 339046 | * After | (index) | Task name | Latency average (ns) | Latency median (ns) | Throughput average (ops/s) | Throughput median (ops/s) | Samples | | ------- | --------------------------------------------------------- | -------------------- | ------------------- | -------------------------- | ------------------------- | ------- | | 0 | 'mark (default)' | '1740.06 ± 1.01%' | '1692.00' | '587400 ± 0.01%' | '591017' | 574695 | | 1 | 'mark (with custom startTime)' | '1661.64 ± 1.16%' | '1612.00' | '617453 ± 0.01%' | '620347' | 601815 | | 2 | 'measure (default)' | '1808.71 ± 1.28%' | '1753.00' | '566516 ± 0.01%' | '570451' | 552882 | | 3 | 'measure (with start and end timestamps)' | '1869.21 ± 1.00%' | '1823.00' | '546571 ± 0.01%' | '548546' | 534987 | | 4 | 'measure (with mark names)' | '2016.40 ± 0.74%' | '1983.00' | '502987 ± 0.01%' | '504286' | 496075 | | 5 | 'clearMarks' | '682.18 ± 0.03%' | '671.00' | '1476364 ± 0.01%' | '1490313' | 1465899 | | 6 | 'clearMeasures' | '686.78 ± 0.03%' | '681.00' | '1467264 ± 0.01%' | '1468429' | 1456081 | | 7 | 'mark + clearMarks' | '2148.90 ± 1.28%' | '2073.00' | '480925 ± 0.01%' | '482393' | 465356 | | 8 | 'measure + clearMeasures (with start and end timestamps)' | '2277.26 ± 1.10%' | '2204.00' | '451016 ± 0.01%' | '453721' | 439125 | | 9 | 'measure + clearMeasures (with mark names)' | '2277.82 ± 0.51%' | '2243.00' | '443853 ± 0.01%' | '445831' | 439016 | Reviewed By: hoxyq Differential Revision: D78193072 fbshipit-source-id: 03b52927a1999f19a2baf0d02335a959525d7add
1 parent 1b7c9ea commit 4edc33e

File tree

9 files changed

+214
-302
lines changed

9 files changed

+214
-302
lines changed

‎packages/react-native/ReactCommon/react/nativemodule/webperformance/NativePerformance.cpp

Lines changed: 17 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -120,108 +120,30 @@ HighResTimeStamp NativePerformance::now(jsi::Runtime& /*rt*/) {
120120
return forcedCurrentTimeStamp_.value_or(HighResTimeStamp::now());
121121
}
122122

123-
HighResTimeStamp NativePerformance::markWithResult(
123+
void NativePerformance::reportMark(
124124
jsi::Runtime& rt,
125125
std::string name,
126-
std::optional<HighResTimeStamp> startTime) {
127-
auto entry = PerformanceEntryReporter::getInstance()->reportMark(
128-
name, startTime.value_or(now(rt)));
129-
return entry.startTime;
130-
}
131-
132-
std::tuple<HighResTimeStamp, HighResDuration> NativePerformance::measure(
133-
jsi::Runtime& runtime,
134-
std::string name,
135-
std::optional<HighResTimeStamp> startTime,
136-
std::optional<HighResTimeStamp> endTime,
137-
std::optional<HighResDuration> duration,
138-
std::optional<std::string> startMark,
139-
std::optional<std::string> endMark) {
140-
auto reporter = PerformanceEntryReporter::getInstance();
141-
142-
HighResTimeStamp startTimeValue;
143-
144-
// If the start time mark name is specified, it takes precedence over the
145-
// startTime parameter, which can be set to 0 by default from JavaScript.
146-
if (startTime) {
147-
startTimeValue = *startTime;
148-
} else if (startMark) {
149-
if (auto startMarkBufferedTime = reporter->getMarkTime(*startMark)) {
150-
startTimeValue = *startMarkBufferedTime;
151-
} else {
152-
throw jsi::JSError(
153-
runtime, "The mark '" + *startMark + "' does not exist.");
154-
}
155-
} else {
156-
startTimeValue = HighResTimeStamp::fromDOMHighResTimeStamp(0);
157-
}
158-
159-
HighResTimeStamp endTimeValue;
160-
161-
if (endTime) {
162-
endTimeValue = *endTime;
163-
} else if (duration) {
164-
endTimeValue = startTimeValue + *duration;
165-
} else if (endMark) {
166-
if (auto endMarkBufferedTime = reporter->getMarkTime(*endMark)) {
167-
endTimeValue = *endMarkBufferedTime;
168-
} else {
169-
throw jsi::JSError(
170-
runtime, "The mark '" + *endMark + "' does not exist.");
171-
}
172-
} else {
173-
// The end time is not specified, take the current time, according to the
174-
// standard
175-
endTimeValue = now(runtime);
176-
}
177-
178-
auto entry = reporter->reportMeasure(name, startTimeValue, endTimeValue);
179-
return std::tuple{entry.startTime, entry.duration};
126+
HighResTimeStamp startTime,
127+
jsi::Value /*entry*/) {
128+
PerformanceEntryReporter::getInstance()->reportMark(name, startTime);
180129
}
181130

182-
std::tuple<HighResTimeStamp, HighResDuration>
183-
NativePerformance::measureWithResult(
184-
jsi::Runtime& runtime,
131+
void NativePerformance::reportMeasure(
132+
jsi::Runtime& rt,
185133
std::string name,
186134
HighResTimeStamp startTime,
187-
HighResTimeStamp endTime,
188-
std::optional<HighResDuration> duration,
189-
std::optional<std::string> startMark,
190-
std::optional<std::string> endMark) {
191-
auto reporter = PerformanceEntryReporter::getInstance();
192-
193-
HighResTimeStamp startTimeValue = startTime;
194-
// If the start time mark name is specified, it takes precedence over the
195-
// startTime parameter, which can be set to 0 by default from JavaScript.
196-
if (startMark) {
197-
if (auto startMarkBufferedTime = reporter->getMarkTime(*startMark)) {
198-
startTimeValue = *startMarkBufferedTime;
199-
} else {
200-
throw jsi::JSError(
201-
runtime, "The mark '" + *startMark + "' does not exist.");
202-
}
203-
}
204-
205-
HighResTimeStamp endTimeValue = endTime;
206-
// If the end time mark name is specified, it takes precedence over the
207-
// startTime parameter, which can be set to 0 by default from JavaScript.
208-
if (endMark) {
209-
if (auto endMarkBufferedTime = reporter->getMarkTime(*endMark)) {
210-
endTimeValue = *endMarkBufferedTime;
211-
} else {
212-
throw jsi::JSError(
213-
runtime, "The mark '" + *endMark + "' does not exist.");
214-
}
215-
} else if (duration) {
216-
endTimeValue = startTimeValue + *duration;
217-
} else if (endTimeValue < startTimeValue) {
218-
// The end time is not specified, take the current time, according to the
219-
// standard
220-
endTimeValue = now(runtime);
221-
}
135+
HighResDuration duration,
136+
jsi::Value /*entry*/) {
137+
PerformanceEntryReporter::getInstance()->reportMeasure(
138+
name, startTime, duration);
139+
}
222140

223-
auto entry = reporter->reportMeasure(name, startTime, endTime);
224-
return std::tuple{entry.startTime, entry.duration};
141+
std::optional<double> NativePerformance::getMarkTime(
142+
jsi::Runtime& rt,
143+
std::string name) {
144+
auto markTime = PerformanceEntryReporter::getInstance()->getMarkTime(name);
145+
return markTime ? std::optional{(*markTime).toDOMHighResTimeStamp()}
146+
: std::nullopt;
225147
}
226148

227149
void NativePerformance::clearMarks(

‎packages/react-native/ReactCommon/react/nativemodule/webperformance/NativePerformance.h

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -90,32 +90,20 @@ class NativePerformance : public NativePerformanceCxxSpec<NativePerformance> {
9090

9191
#pragma mark - User Timing Level 3 functions (https://w3c.github.io/user-timing/)
9292

93-
// https://w3c.github.io/user-timing/#mark-method
94-
HighResTimeStamp markWithResult(
93+
void reportMark(
9594
jsi::Runtime& rt,
9695
std::string name,
97-
std::optional<HighResTimeStamp> startTime);
96+
HighResTimeStamp time,
97+
jsi::Value entry);
9898

99-
// https://w3c.github.io/user-timing/#measure-method
100-
std::tuple<HighResTimeStamp, HighResDuration> measure(
101-
jsi::Runtime& rt,
102-
std::string name,
103-
std::optional<HighResTimeStamp> startTime,
104-
std::optional<HighResTimeStamp> endTime,
105-
std::optional<HighResDuration> duration,
106-
std::optional<std::string> startMark,
107-
std::optional<std::string> endMark);
108-
109-
// https://w3c.github.io/user-timing/#measure-method
110-
[[deprecated("This method is deprecated. Use the measure method instead.")]]
111-
std::tuple<HighResTimeStamp, HighResDuration> measureWithResult(
99+
void reportMeasure(
112100
jsi::Runtime& rt,
113101
std::string name,
114102
HighResTimeStamp startTime,
115-
HighResTimeStamp endTime,
116-
std::optional<HighResDuration> duration,
117-
std::optional<std::string> startMark,
118-
std::optional<std::string> endMark);
103+
HighResDuration duration,
104+
jsi::Value entry);
105+
106+
std::optional<double> getMarkTime(jsi::Runtime& rt, std::string name);
119107

120108
// https://w3c.github.io/user-timing/#clearmarks-method
121109
void clearMarks(

‎packages/react-native/ReactCommon/react/performance/timeline/PerformanceEntryReporter.cpp

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,10 @@ void PerformanceEntryReporter::clearEntries(
167167
getBufferRef(entryType).clear(entryName);
168168
}
169169

170-
PerformanceMark PerformanceEntryReporter::reportMark(
170+
void PerformanceEntryReporter::reportMark(
171171
const std::string& name,
172-
const std::optional<HighResTimeStamp>& startTime) {
173-
// Resolve timings
174-
auto startTimeVal = startTime ? *startTime : getCurrentTimeStamp();
175-
const auto entry = PerformanceMark{{.name = name, .startTime = startTimeVal}};
172+
const HighResTimeStamp startTime) {
173+
const auto entry = PerformanceMark{{.name = name, .startTime = startTime}};
176174

177175
traceMark(entry);
178176

@@ -183,18 +181,14 @@ PerformanceMark PerformanceEntryReporter::reportMark(
183181
}
184182

185183
observerRegistry_->queuePerformanceEntry(entry);
186-
187-
return entry;
188184
}
189185

190-
PerformanceMeasure PerformanceEntryReporter::reportMeasure(
186+
void PerformanceEntryReporter::reportMeasure(
191187
const std::string& name,
192188
HighResTimeStamp startTime,
193-
HighResTimeStamp endTime,
189+
HighResDuration duration,
194190
const std::optional<jsinspector_modern::DevToolsTrackEntryPayload>&
195191
trackMetadata) {
196-
HighResDuration duration = endTime - startTime;
197-
198192
const auto entry = PerformanceMeasure{
199193
{.name = std::string(name),
200194
.startTime = startTime,
@@ -209,8 +203,6 @@ PerformanceMeasure PerformanceEntryReporter::reportMeasure(
209203
}
210204

211205
observerRegistry_->queuePerformanceEntry(entry);
212-
213-
return entry;
214206
}
215207

216208
void PerformanceEntryReporter::clearEventCounts() {
@@ -275,7 +267,7 @@ void PerformanceEntryReporter::reportLongTask(
275267
observerRegistry_->queuePerformanceEntry(entry);
276268
}
277269

278-
PerformanceResourceTiming PerformanceEntryReporter::reportResourceTiming(
270+
void PerformanceEntryReporter::reportResourceTiming(
279271
const std::string& url,
280272
HighResTimeStamp fetchStart,
281273
HighResTimeStamp requestStart,
@@ -302,8 +294,6 @@ PerformanceResourceTiming PerformanceEntryReporter::reportResourceTiming(
302294
}
303295

304296
observerRegistry_->queuePerformanceEntry(entry);
305-
306-
return entry;
307297
}
308298

309299
void PerformanceEntryReporter::traceMark(const PerformanceMark& entry) const {

‎packages/react-native/ReactCommon/react/performance/timeline/PerformanceEntryReporter.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,12 @@ class PerformanceEntryReporter {
8686
std::optional<HighResTimeStamp> getMarkTime(
8787
const std::string& markName) const;
8888

89-
PerformanceMark reportMark(
90-
const std::string& name,
91-
const std::optional<HighResTimeStamp>& startTime = std::nullopt);
89+
void reportMark(const std::string& name, HighResTimeStamp startTime);
9290

93-
PerformanceMeasure reportMeasure(
91+
void reportMeasure(
9492
const std::string& name,
9593
HighResTimeStamp startTime,
96-
HighResTimeStamp endTime,
94+
HighResDuration duration,
9795
const std::optional<jsinspector_modern::DevToolsTrackEntryPayload>&
9896
trackMetadata = std::nullopt);
9997

@@ -107,7 +105,7 @@ class PerformanceEntryReporter {
107105

108106
void reportLongTask(HighResTimeStamp startTime, HighResDuration duration);
109107

110-
PerformanceResourceTiming reportResourceTiming(
108+
void reportResourceTiming(
111109
const std::string& url,
112110
HighResTimeStamp fetchStart,
113111
HighResTimeStamp requestStart,

‎packages/react-native/ReactCommon/react/performance/timeline/tests/PerformanceEntryReporterTest.cpp

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -129,20 +129,16 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
129129
"mark2", timeOrigin + HighResDuration::fromMilliseconds(2));
130130

131131
reporter->reportMeasure(
132-
"measure0",
133-
timeOrigin,
134-
timeOrigin + HighResDuration::fromMilliseconds(2));
132+
"measure0", timeOrigin, HighResDuration::fromMilliseconds(2));
135133
reporter->reportMeasure(
136-
"measure1",
137-
timeOrigin,
138-
timeOrigin + HighResDuration::fromMilliseconds(3));
134+
"measure1", timeOrigin, HighResDuration::fromMilliseconds(3));
139135

140136
reporter->reportMark(
141137
"mark3", timeOrigin + HighResDuration::fromNanoseconds(2.5 * 1e6));
142138
reporter->reportMeasure(
143139
"measure2",
144140
timeOrigin + HighResDuration::fromMilliseconds(2),
145-
timeOrigin + HighResDuration::fromMilliseconds(2));
141+
HighResDuration::fromMilliseconds(2));
146142
reporter->reportMark(
147143
"mark4", timeOrigin + HighResDuration::fromMilliseconds(3));
148144

@@ -172,7 +168,7 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
172168
PerformanceMeasure{
173169
{.name = "measure2",
174170
.startTime = timeOrigin + HighResDuration::fromMilliseconds(2),
175-
.duration = HighResDuration::zero()}},
171+
.duration = HighResDuration::fromMilliseconds(2)}},
176172
PerformanceMark{
177173
{.name = "mark3",
178174
.startTime =
@@ -203,21 +199,17 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestGetEntries) {
203199
"mark2", timeOrigin + HighResDuration::fromMilliseconds(2));
204200

205201
reporter->reportMeasure(
206-
"common_name",
207-
timeOrigin,
208-
timeOrigin + HighResDuration::fromMilliseconds(2));
202+
"common_name", timeOrigin, HighResDuration::fromMilliseconds(2));
209203
reporter->reportMeasure(
210-
"measure1",
211-
timeOrigin,
212-
timeOrigin + HighResDuration::fromMilliseconds(3));
204+
"measure1", timeOrigin, HighResDuration::fromMilliseconds(3));
213205
reporter->reportMeasure(
214206
"measure2",
215207
timeOrigin + HighResDuration::fromMilliseconds(1),
216-
timeOrigin + HighResDuration::fromMilliseconds(6));
208+
HighResDuration::fromMilliseconds(6));
217209
reporter->reportMeasure(
218210
"measure3",
219211
timeOrigin + HighResDuration::fromNanoseconds(1.5 * 1e6),
220-
timeOrigin + HighResDuration::fromMilliseconds(2));
212+
HighResDuration::fromMilliseconds(2));
221213

222214
{
223215
const auto allEntries = toSorted(reporter->getEntries());
@@ -241,12 +233,12 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestGetEntries) {
241233
PerformanceMeasure{
242234
{.name = "measure2",
243235
.startTime = timeOrigin + HighResDuration::fromMilliseconds(1),
244-
.duration = HighResDuration::fromMilliseconds(5)}},
236+
.duration = HighResDuration::fromMilliseconds(6)}},
245237
PerformanceMeasure{
246238
{.name = "measure3",
247239
.startTime =
248240
timeOrigin + HighResDuration::fromNanoseconds(1.5 * 1e6),
249-
.duration = HighResDuration::fromNanoseconds(0.5 * 1e6)}},
241+
.duration = HighResDuration::fromMilliseconds(2)}},
250242
PerformanceMark{
251243
{.name = "mark2",
252244
.startTime = timeOrigin + HighResDuration::fromMilliseconds(2),
@@ -288,12 +280,12 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestGetEntries) {
288280
PerformanceMeasure{
289281
{.name = "measure2",
290282
.startTime = timeOrigin + HighResDuration::fromMilliseconds(1),
291-
.duration = HighResDuration::fromMilliseconds(5)}},
283+
.duration = HighResDuration::fromMilliseconds(6)}},
292284
PerformanceMeasure{
293285
{.name = "measure3",
294286
.startTime =
295287
timeOrigin + HighResDuration::fromNanoseconds(1.5 * 1e6),
296-
.duration = HighResDuration::fromNanoseconds(0.5 * 1e6)}}};
288+
.duration = HighResDuration::fromMilliseconds(2)}}};
297289
ASSERT_EQ(expected, measures);
298290
}
299291

@@ -330,21 +322,17 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestClearMarks) {
330322
"mark2", timeOrigin + HighResDuration::fromMilliseconds(2));
331323

332324
reporter->reportMeasure(
333-
"common_name",
334-
timeOrigin,
335-
timeOrigin + HighResDuration::fromMilliseconds(2));
325+
"common_name", timeOrigin, HighResDuration::fromMilliseconds(2));
336326
reporter->reportMeasure(
337-
"measure1",
338-
timeOrigin,
339-
timeOrigin + HighResDuration::fromMilliseconds(3));
327+
"measure1", timeOrigin, HighResDuration::fromMilliseconds(3));
340328
reporter->reportMeasure(
341329
"measure2",
342330
timeOrigin + HighResDuration::fromMilliseconds(1),
343-
timeOrigin + HighResDuration::fromMilliseconds(6));
331+
HighResDuration::fromMilliseconds(6));
344332
reporter->reportMeasure(
345333
"measure3",
346334
timeOrigin + HighResDuration::fromNanoseconds(1.5 * 1e6),
347-
timeOrigin + HighResDuration::fromMilliseconds(2));
335+
HighResDuration::fromMilliseconds(2));
348336

349337
reporter->clearEntries(PerformanceEntryType::MARK, "common_name");
350338

‎packages/react-native/ReactCommon/react/performance/timeline/tests/PerformanceObserverTest.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ TEST(PerformanceObserver, PerformanceObserverTestObserveTakeRecords) {
190190
reporter->reportMeasure(
191191
"off",
192192
timeOrigin + HighResDuration::fromMilliseconds(10),
193-
timeOrigin + HighResDuration::fromMilliseconds(20));
193+
HighResDuration::fromMilliseconds(20));
194194
reporter->reportMark(
195195
"test2", timeOrigin + HighResDuration::fromMilliseconds(20));
196196
reporter->reportMark(
@@ -371,9 +371,7 @@ TEST(PerformanceObserver, PerformanceObserverTestMultiple) {
371371
{.durationThreshold = HighResDuration::fromMilliseconds(80)});
372372

373373
reporter->reportMeasure(
374-
"measure",
375-
timeOrigin,
376-
timeOrigin + HighResDuration::fromMilliseconds(50));
374+
"measure", timeOrigin, HighResDuration::fromMilliseconds(50));
377375
reporter->reportEvent(
378376
"event1",
379377
timeOrigin,

0 commit comments

Comments
 (0)