Skip to content

Commit 4475572

Browse files
rshestfacebook-github-bot
authored andcommitted
Performance.measure: handle the case with a single startMark argument correctly (#37888)
Summary: Pull Request resolved: #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
1 parent 377a8b7 commit 4475572

File tree

3 files changed

+34
-6
lines changed

3 files changed

+34
-6
lines changed

packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@
1515
namespace facebook::react {
1616
EventTag PerformanceEntryReporter::sCurrentEventTag_{0};
1717

18-
static inline double getCurrentTimeStamp() {
19-
return JSExecutor::performanceNow();
20-
}
21-
2218
PerformanceEntryReporter &PerformanceEntryReporter::getInstance() {
2319
static PerformanceEntryReporter instance;
2420
return instance;
@@ -29,11 +25,17 @@ PerformanceEntryReporter::PerformanceEntryReporter() {
2925
// sure that marks can be referenced by measures
3026
getBuffer(PerformanceEntryType::MARK).hasNameLookup = true;
3127
}
28+
3229
void PerformanceEntryReporter::setReportingCallback(
3330
std::optional<AsyncCallback<>> callback) {
3431
callback_ = callback;
3532
}
3633

34+
double PerformanceEntryReporter::getCurrentTimeStamp() const {
35+
return timeStampProvider_ != nullptr ? timeStampProvider_()
36+
: JSExecutor::performanceNow();
37+
}
38+
3739
void PerformanceEntryReporter::startReporting(PerformanceEntryType entryType) {
3840
auto &buffer = getBuffer(entryType);
3941
buffer.isReporting = true;
@@ -219,7 +221,15 @@ void PerformanceEntryReporter::measure(
219221
const std::optional<std::string> &endMark) {
220222
double startTimeVal = startMark ? getMarkTime(*startMark) : startTime;
221223
double endTimeVal = endMark ? getMarkTime(*endMark) : endTime;
224+
225+
if (!endMark && endTime < startTimeVal) {
226+
// The end time is not specified, take the current time, according to the
227+
// standard
228+
endTimeVal = getCurrentTimeStamp();
229+
}
230+
222231
double durationVal = duration ? *duration : endTimeVal - startTimeVal;
232+
223233
logEntry(
224234
{name,
225235
static_cast<int>(PerformanceEntryType::MEASURE),

packages/react-native/Libraries/WebPerformance/PerformanceEntryReporter.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ class PerformanceEntryReporter : public EventLogger {
147147
return eventCounts_;
148148
}
149149

150+
void setTimeStampProvider(std::function<double()> provider) {
151+
timeStampProvider_ = provider;
152+
}
153+
150154
private:
151155
std::optional<AsyncCallback<>> callback_;
152156

@@ -171,6 +175,8 @@ class PerformanceEntryReporter : public EventLogger {
171175
std::unordered_map<EventTag, EventEntry> eventsInFlight_;
172176
std::mutex eventsInFlightMutex_;
173177

178+
std::function<double()> timeStampProvider_ = nullptr;
179+
174180
static EventTag sCurrentEventTag_;
175181

176182
PerformanceEntryReporter();
@@ -182,6 +188,8 @@ class PerformanceEntryReporter : public EventLogger {
182188
PerformanceEntryType entryType,
183189
const char *entryName,
184190
std::vector<RawPerformanceEntry> &res) const;
191+
192+
double getCurrentTimeStamp() const;
185193
};
186194

187195
} // namespace facebook::react

packages/react-native/Libraries/WebPerformance/__tests__/PerformanceEntryReporterTest.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,11 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
146146
reporter.measure("measure3", 0.0, 0.0, 5.0, "mark1");
147147
reporter.measure("measure4", 1.5, 0.0, std::nullopt, std::nullopt, "mark2");
148148

149+
reporter.setTimeStampProvider([]() { return 3.5; });
150+
reporter.measure("measure5", 0.0, 0.0, std::nullopt, "mark2");
151+
149152
reporter.mark("mark3", 2.0);
150-
reporter.measure("measure5", 2.0, 2.0);
153+
reporter.measure("measure6", 2.0, 2.0);
151154
reporter.mark("mark4", 2.0);
152155

153156
auto res = reporter.popPendingEntries();
@@ -226,13 +229,20 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
226229
std::nullopt,
227230
std::nullopt,
228231
std::nullopt},
229-
{"measure5",
232+
{"measure6",
230233
static_cast<int>(PerformanceEntryType::MEASURE),
231234
2.0,
232235
0.0,
233236
std::nullopt,
234237
std::nullopt,
235238
std::nullopt},
239+
{"measure5",
240+
static_cast<int>(PerformanceEntryType::MEASURE),
241+
2.0,
242+
1.5,
243+
std::nullopt,
244+
std::nullopt,
245+
std::nullopt},
236246
};
237247

238248
ASSERT_EQ(expected, entries);

0 commit comments

Comments
 (0)