Skip to content

Commit f6ecbf7

Browse files
hoxyqfacebook-github-bot
authored andcommitted
Avoid copying strings when serializing TraceEvent / lock only on buffer operations (facebook#52220)
Summary: # Changelog: [Internal] Mainly, 2 changes: 1. `PerformanceTracer::serializeTraceEvent(const TraceEvent& event)` -> `PerformanceTracer::serializeTraceEvent(TraceEvent&& event)` for less copies, actually move strings from the `TraceEvent` into the serialized `folly:object`. 2. When collecting events from the buffer, only lock when accessing buffer, not when serializing. Reviewed By: rubennorte Differential Revision: D77164969
1 parent 6454623 commit f6ecbf7

File tree

2 files changed

+26
-17
lines changed

2 files changed

+26
-17
lines changed

packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,27 +87,30 @@ void PerformanceTracer::collectEvents(
8787
const std::function<void(const folly::dynamic& eventsChunk)>&
8888
resultCallback,
8989
uint16_t chunkSize) {
90-
std::lock_guard lock(tracingMutex_);
90+
std::vector<TraceEvent> localBuffer;
91+
{
92+
std::lock_guard lock(tracingMutex_);
93+
localBuffer = std::move(buffer_);
94+
buffer_.clear();
95+
}
9196

92-
if (buffer_.empty()) {
97+
if (localBuffer.empty()) {
9398
return;
9499
}
95100

96-
auto traceEvents = folly::dynamic::array();
97-
for (const auto& event : buffer_) {
101+
auto serializedTraceEvents = folly::dynamic::array();
102+
for (auto&& event : localBuffer) {
98103
// Emit trace events
99-
traceEvents.push_back(serializeTraceEvent(event));
104+
serializedTraceEvents.push_back(serializeTraceEvent(std::move(event)));
100105

101-
if (traceEvents.size() == chunkSize) {
102-
resultCallback(traceEvents);
103-
traceEvents = folly::dynamic::array();
106+
if (serializedTraceEvents.size() == chunkSize) {
107+
resultCallback(serializedTraceEvents);
108+
serializedTraceEvents = folly::dynamic::array();
104109
}
105110
}
106-
if (!traceEvents.empty()) {
107-
resultCallback(traceEvents);
111+
if (!serializedTraceEvents.empty()) {
112+
resultCallback(serializedTraceEvents);
108113
}
109-
110-
buffer_.clear();
111114
}
112115

113116
void PerformanceTracer::reportMark(
@@ -326,21 +329,21 @@ folly::dynamic PerformanceTracer::getSerializedRuntimeProfileChunkTraceEvent(
326329
}
327330

328331
folly::dynamic PerformanceTracer::serializeTraceEvent(
329-
const TraceEvent& event) const {
332+
TraceEvent&& event) const {
330333
folly::dynamic result = folly::dynamic::object;
331334

332335
if (event.id.has_value()) {
333336
std::array<char, 16> buffer{};
334337
snprintf(buffer.data(), buffer.size(), "0x%x", event.id.value());
335338
result["id"] = buffer.data();
336339
}
337-
result["name"] = event.name;
338-
result["cat"] = event.cat;
340+
result["name"] = std::move(event.name);
341+
result["cat"] = std::move(event.cat);
339342
result["ph"] = std::string(1, event.ph);
340343
result["ts"] = highResTimeStampToTracingClockTimeStamp(event.ts);
341344
result["pid"] = event.pid;
342345
result["tid"] = event.tid;
343-
result["args"] = event.args;
346+
result["args"] = std::move(event.args);
344347
if (event.dur.has_value()) {
345348
result["dur"] = highResDurationToTracingClockDuration(event.dur.value());
346349
}

packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,13 @@ class PerformanceTracer {
132132
PerformanceTracer& operator=(const PerformanceTracer&) = delete;
133133
~PerformanceTracer() = default;
134134

135-
folly::dynamic serializeTraceEvent(const TraceEvent& event) const;
135+
/**
136+
* Serialize a TraceEvent into a folly::dynamic object.
137+
* \param event rvalue reference to the TraceEvent object.
138+
* \return folly::dynamic object that represents a serialized into JSON Trace
139+
* Event for CDP.
140+
*/
141+
folly::dynamic serializeTraceEvent(TraceEvent&& event) const;
136142

137143
uint64_t processId_;
138144

0 commit comments

Comments
 (0)