From 971fb6453c51f85d1d38675133fcc0cdbe79653d Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Fri, 4 Apr 2025 14:06:40 -0700 Subject: [PATCH] Disallow copying RuntimeSamplingProfile and Sample Summary: # Changelog: [Internal] We should avoid allocating multiple of these, since they can get really big. `RuntimeSamplingProfile` may contain up to 10k of `Sample` for every second of sampling. Every `Sample` may contain up to 500 of call frames, each of which may have a copy of strings like `functionName` and `url`. > NOTE: The actual logic for deduplicating strings and not allocating too many of them is not part of this diff and will be published separately. Even with deduplication, there is a reason to avoid copying the profile. Reviewed By: javache Differential Revision: D72254364 fbshipit-source-id: fa7907fb5cabbd2ac99d3e4e05a93bcb07f7f7c7 --- .../jsinspector-modern/InstanceAgent.cpp | 2 +- .../tracing/InstanceTracingProfile.h | 7 ++----- .../tracing/RuntimeSamplingProfile.h | 16 ++++++++++++++++ ...untimeSamplingProfileTraceEventSerializer.cpp | 8 ++++---- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp index 1af9f9448c9..7b2cd6c7734 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/InstanceAgent.cpp @@ -170,7 +170,7 @@ tracing::InstanceTracingProfile InstanceAgent::collectTracingProfile() { tracing::RuntimeSamplingProfile runtimeSamplingProfile = runtimeAgent_->collectSamplingProfile(); - return tracing::InstanceTracingProfile{runtimeSamplingProfile}; + return tracing::InstanceTracingProfile{std::move(runtimeSamplingProfile)}; } } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/InstanceTracingProfile.h b/packages/react-native/ReactCommon/jsinspector-modern/tracing/InstanceTracingProfile.h index 4e23beb7618..e4045ff54e7 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/InstanceTracingProfile.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/InstanceTracingProfile.h @@ -9,15 +9,12 @@ #include "RuntimeSamplingProfile.h" -#include - namespace facebook::react::jsinspector_modern::tracing { struct InstanceTracingProfile { public: - explicit InstanceTracingProfile( - const RuntimeSamplingProfile runtimeSamplingProfile) - : runtimeSamplingProfile_(runtimeSamplingProfile) {} + explicit InstanceTracingProfile(RuntimeSamplingProfile runtimeSamplingProfile) + : runtimeSamplingProfile_(std::move(runtimeSamplingProfile)) {} const RuntimeSamplingProfile& getRuntimeSamplingProfile() const { return runtimeSamplingProfile_; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfile.h b/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfile.h index aeb86d8948c..e6373029b06 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfile.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfile.h @@ -113,6 +113,14 @@ struct RuntimeSamplingProfile { threadId_(threadId), callStack_(std::move(callStack)) {} + // Movable. + Sample& operator=(Sample&&) = default; + Sample(Sample&&) = default; + + // Not copyable. + Sample(const Sample&) = delete; + Sample& operator=(const Sample&) = delete; + /// \return serialized unix timestamp in microseconds granularity. The /// moment when this sample was recorded. uint64_t getTimestamp() const { @@ -143,6 +151,14 @@ struct RuntimeSamplingProfile { RuntimeSamplingProfile(std::string runtimeName, std::vector samples) : runtimeName_(std::move(runtimeName)), samples_(std::move(samples)) {} + // Movable. + RuntimeSamplingProfile& operator=(RuntimeSamplingProfile&&) = default; + RuntimeSamplingProfile(RuntimeSamplingProfile&&) = default; + + // Not copyable. + RuntimeSamplingProfile(const RuntimeSamplingProfile&) = delete; + RuntimeSamplingProfile& operator=(const RuntimeSamplingProfile&) = delete; + /// \return name of the JavaScript runtime, where sampling occurred. const std::string& getRuntimeName() const { return runtimeName_; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.cpp index c203e6da85e..573d4c566cb 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.cpp @@ -87,15 +87,15 @@ RuntimeSamplingProfileTraceEventSerializer::serializeAndNotify( notificationCallback, uint16_t traceEventChunkSize, uint16_t profileChunkSize) { - std::vector runtimeSamples = + const std::vector& samples = profile.getSamples(); - if (runtimeSamples.empty()) { + if (samples.empty()) { return; } std::vector buffer; - uint64_t chunkThreadId = runtimeSamples.front().getThreadId(); + uint64_t chunkThreadId = samples.front().getThreadId(); uint64_t tracingStartUnixTimestamp = formatTimePointToUnixTimestamp(tracingStartTime); buffer.push_back(performanceTracer.getSerializedRuntimeProfileTraceEvent( @@ -151,7 +151,7 @@ RuntimeSamplingProfileTraceEventSerializer::serializeAndNotify( 0, "(garbage collector)"}; uint64_t chunkTimestamp = tracingStartUnixTimestamp; - for (const RuntimeSamplingProfile::Sample& sample : runtimeSamples) { + for (const RuntimeSamplingProfile::Sample& sample : samples) { uint64_t sampleThreadId = sample.getThreadId(); // If next sample was recorded on a different thread, emit the current chunk // and continue.