diff --git a/mozglue/baseprofiler/core/platform.cpp b/mozglue/baseprofiler/core/platform.cpp index 6218ae2753e5..60ede5bb1f0a 100644 --- a/mozglue/baseprofiler/core/platform.cpp +++ b/mozglue/baseprofiler/core/platform.cpp @@ -3198,17 +3198,19 @@ void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) { delete aBacktrace; } -static void racy_profiler_add_marker( - const char* aMarkerName, ProfilingCategoryPair aCategoryPair, - UniquePtr aPayload) { +static void racy_profiler_add_marker(const char* aMarkerName, + ProfilingCategoryPair aCategoryPair, + const ProfilerMarkerPayload* aPayload) { MOZ_RELEASE_ASSERT(CorePS::Exists()); - // We don't assert that RacyFeatures::IsActiveWithoutPrivacy() or - // RacyRegisteredThread::IsBeingProfiled() is true here, because it's - // possible that the result has changed since we tested it in the caller. - // - // Because of this imprecision it's possible to miss a marker or record one - // we shouldn't. Either way is not a big deal. + // This function is hot enough that we use RacyFeatures, not ActivePS. + if (!RacyFeatures::IsActiveWithoutPrivacy()) { + return; + } + + // Note that it's possible that the above test would change again before we + // actually record the marker. Because of this imprecision it's possible to + // miss a marker or record one we shouldn't. Either way is not a big deal. RacyRegisteredThread* racyRegisteredThread = TLSRegisteredThread::RacyRegisteredThread(); @@ -3228,27 +3230,20 @@ static void racy_profiler_add_marker( void profiler_add_marker(const char* aMarkerName, ProfilingCategoryPair aCategoryPair, - UniquePtr aPayload) { - MOZ_RELEASE_ASSERT(CorePS::Exists()); - - // This function is hot enough that we use RacyFeatures, not ActivePS. - if (!RacyFeatures::IsActiveWithoutPrivacy()) { - return; - } - - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(aPayload)); + const ProfilerMarkerPayload& aPayload) { + racy_profiler_add_marker(aMarkerName, aCategoryPair, &aPayload); } void profiler_add_marker(const char* aMarkerName, ProfilingCategoryPair aCategoryPair) { - profiler_add_marker(aMarkerName, aCategoryPair, nullptr); + racy_profiler_add_marker(aMarkerName, aCategoryPair, nullptr); } // This is a simplified version of profiler_add_marker that can be easily passed // into the JS engine. void profiler_add_js_marker(const char* aMarkerName) { AUTO_PROFILER_STATS(base_add_marker); - profiler_add_marker(aMarkerName, ProfilingCategoryPair::JS, nullptr); + profiler_add_marker(aMarkerName, ProfilingCategoryPair::JS); } // This logic needs to add a marker for a different thread, so we actually need @@ -3306,9 +3301,9 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName, } AUTO_PROFILER_STATS(base_add_marker_with_TracingMarkerPayload); - auto payload = MakeUnique( - aCategoryString, aKind, aDocShellId, aDocShellHistoryId); - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload)); + profiler_add_marker(aMarkerName, aCategoryPair, + TracingMarkerPayload(aCategoryString, aKind, aDocShellId, + aDocShellHistoryId)); } void profiler_tracing(const char* aCategoryString, const char* aMarkerName, @@ -3326,10 +3321,10 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName, } AUTO_PROFILER_STATS(base_add_marker_with_TracingMarkerPayload); - auto payload = - MakeUnique(aCategoryString, aKind, aDocShellId, - aDocShellHistoryId, std::move(aCause)); - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload)); + profiler_add_marker( + aMarkerName, aCategoryPair, + TracingMarkerPayload(aCategoryString, aKind, aDocShellId, + aDocShellHistoryId, std::move(aCause))); } void profiler_add_text_marker(const char* aMarkerName, const std::string& aText, @@ -3342,8 +3337,8 @@ void profiler_add_text_marker(const char* aMarkerName, const std::string& aText, AUTO_PROFILER_STATS(base_add_marker_with_TextMarkerPayload); profiler_add_marker( aMarkerName, aCategoryPair, - MakeUnique(aText, aStartTime, aEndTime, aDocShellId, - aDocShellHistoryId, std::move(aCause))); + TextMarkerPayload(aText, aStartTime, aEndTime, aDocShellId, + aDocShellHistoryId, std::move(aCause))); } // NOTE: aCollector's methods will be called while the target thread is paused. diff --git a/mozglue/baseprofiler/public/BaseProfiler.h b/mozglue/baseprofiler/public/BaseProfiler.h index eb1b9e797b5b..60ee4d1eabd4 100644 --- a/mozglue/baseprofiler/public/BaseProfiler.h +++ b/mozglue/baseprofiler/public/BaseProfiler.h @@ -712,12 +712,13 @@ MFBT_API void profiler_add_marker(const char* aMarkerName, ::mozilla::baseprofiler::profiler_add_marker( \ markerName, \ ::mozilla::baseprofiler::ProfilingCategoryPair::categoryPair, \ - ::mozilla::MakeUnique parenthesizedPayloadArgs); \ + PayloadType parenthesizedPayloadArgs); \ } while (false) MFBT_API void profiler_add_marker(const char* aMarkerName, ProfilingCategoryPair aCategoryPair, - UniquePtr aPayload); + const ProfilerMarkerPayload& aPayload); + MFBT_API void profiler_add_js_marker(const char* aMarkerName); // Insert a marker in the profile timeline for a specified thread. diff --git a/mozglue/baseprofiler/public/BaseProfilerMarkerPayload.h b/mozglue/baseprofiler/public/BaseProfilerMarkerPayload.h index ba0cebac8d58..cf864911bad9 100644 --- a/mozglue/baseprofiler/public/BaseProfilerMarkerPayload.h +++ b/mozglue/baseprofiler/public/BaseProfilerMarkerPayload.h @@ -333,6 +333,21 @@ class LogMarkerPayload : public ProfilerMarkerPayload { // Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no // payloads. template <> +struct BlocksRingBuffer::Serializer< + const baseprofiler::ProfilerMarkerPayload*> { + static Length Bytes(const baseprofiler::ProfilerMarkerPayload* aPayload) { + return baseprofiler::ProfilerMarkerPayload::TagAndSerializationBytes( + aPayload); + } + + static void Write(EntryWriter& aEW, + const baseprofiler::ProfilerMarkerPayload* aPayload) { + baseprofiler::ProfilerMarkerPayload::TagAndSerialize(aPayload, aEW); + } +}; + +// Serialize a pointed-at ProfilerMarkerPayload, may be null for no payloads. +template <> struct BlocksRingBuffer::Serializer< UniquePtr> { static Length Bytes( diff --git a/mozglue/tests/TestBaseProfiler.cpp b/mozglue/tests/TestBaseProfiler.cpp index 9f4b4bdcd892..3647c4b357d0 100644 --- a/mozglue/tests/TestBaseProfiler.cpp +++ b/mozglue/tests/TestBaseProfiler.cpp @@ -1475,10 +1475,9 @@ void TestProfilerMarkerSerialization() { constexpr int data = 42; { - UniquePtr testPayload{ - new BaseTestMarkerPayload(data)}; - - rb.PutObject(testPayload); + BaseTestMarkerPayload payload(data); + rb.PutObject( + static_cast(&payload)); } int read = 0; @@ -1644,8 +1643,8 @@ void TestProfiler() { // Just making sure all payloads know how to (de)serialize and stream. baseprofiler::profiler_add_marker( "TracingMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique( - "category", baseprofiler::TRACING_EVENT)); + baseprofiler::TracingMarkerPayload("category", + baseprofiler::TRACING_EVENT)); auto cause = # if defined(__linux__) || defined(__ANDROID__) @@ -1656,37 +1655,37 @@ void TestProfiler() { # endif baseprofiler::profiler_add_marker( "FileIOMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique( + baseprofiler::FileIOMarkerPayload( "operation", "source", "filename", TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed(), std::move(cause))); baseprofiler::profiler_add_marker( "UserTimingMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique( - "name", TimeStamp::NowUnfuzzed(), Nothing{}, Nothing{})); + baseprofiler::UserTimingMarkerPayload("name", TimeStamp::NowUnfuzzed(), + Nothing{}, Nothing{})); baseprofiler::profiler_add_marker( "HangMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique(TimeStamp::NowUnfuzzed(), - TimeStamp::NowUnfuzzed())); + baseprofiler::HangMarkerPayload(TimeStamp::NowUnfuzzed(), + TimeStamp::NowUnfuzzed())); baseprofiler::profiler_add_marker( "LongTaskMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique( - TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed())); + baseprofiler::LongTaskMarkerPayload(TimeStamp::NowUnfuzzed(), + TimeStamp::NowUnfuzzed())); { std::string s = "text payload"; baseprofiler::profiler_add_marker( "TextMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique( - s, TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed())); + baseprofiler::TextMarkerPayload(s, TimeStamp::NowUnfuzzed(), + TimeStamp::NowUnfuzzed())); } baseprofiler::profiler_add_marker( "LogMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, - MakeUnique("module", "text", - TimeStamp::NowUnfuzzed())); + baseprofiler::LogMarkerPayload("module", "text", + TimeStamp::NowUnfuzzed())); printf("Sleep 1s...\n"); { diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index 9c6de2bcb021..941305487461 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -4067,17 +4067,19 @@ void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) { delete aBacktrace; } -static void racy_profiler_add_marker( - const char* aMarkerName, JS::ProfilingCategoryPair aCategoryPair, - UniquePtr aPayload) { +static void racy_profiler_add_marker(const char* aMarkerName, + JS::ProfilingCategoryPair aCategoryPair, + const ProfilerMarkerPayload* aPayload) { MOZ_RELEASE_ASSERT(CorePS::Exists()); - // We don't assert that RacyFeatures::IsActiveWithoutPrivacy() or - // RacyRegisteredThread::IsBeingProfiled() is true here, because it's - // possible that the result has changed since we tested it in the caller. - // - // Because of this imprecision it's possible to miss a marker or record one - // we shouldn't. Either way is not a big deal. + // This function is hot enough that we use RacyFeatures, not ActivePS. + if (!RacyFeatures::IsActiveWithoutPrivacy()) { + return; + } + + // Note that it's possible that the above test would change again before we + // actually record the marker. Because of this imprecision it's possible to + // miss a marker or record one we shouldn't. Either way is not a big deal. RacyRegisteredThread* racyRegisteredThread = TLSRegisteredThread::RacyRegisteredThread(); @@ -4097,35 +4099,28 @@ static void racy_profiler_add_marker( void profiler_add_marker(const char* aMarkerName, JS::ProfilingCategoryPair aCategoryPair, - UniquePtr aPayload) { - MOZ_RELEASE_ASSERT(CorePS::Exists()); - - // This function is hot enough that we use RacyFeatures, not ActivePS. - if (!RacyFeatures::IsActiveWithoutPrivacy()) { - return; - } - - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(aPayload)); + const ProfilerMarkerPayload& aPayload) { + racy_profiler_add_marker(aMarkerName, aCategoryPair, &aPayload); } void profiler_add_marker(const char* aMarkerName, JS::ProfilingCategoryPair aCategoryPair) { - profiler_add_marker(aMarkerName, aCategoryPair, nullptr); + racy_profiler_add_marker(aMarkerName, aCategoryPair, nullptr); } // This is a simplified version of profiler_add_marker that can be easily passed // into the JS engine. void profiler_add_js_marker(const char* aMarkerName) { AUTO_PROFILER_STATS(add_marker); - profiler_add_marker(aMarkerName, JS::ProfilingCategoryPair::JS, nullptr); + profiler_add_marker(aMarkerName, JS::ProfilingCategoryPair::JS); } void profiler_add_js_allocation_marker(JS::RecordAllocationInfo&& info) { AUTO_PROFILER_STATS(add_marker_with_JsAllocationMarkerPayload); profiler_add_marker( "JS allocation", JS::ProfilingCategoryPair::JS, - MakeUnique(TimeStamp::Now(), std::move(info), - profiler_get_backtrace())); + JsAllocationMarkerPayload(TimeStamp::Now(), std::move(info), + profiler_get_backtrace())); } void profiler_add_network_marker( @@ -4153,7 +4148,7 @@ void profiler_add_network_marker( AUTO_PROFILER_STATS(add_marker_with_NetworkMarkerPayload); profiler_add_marker( name, JS::ProfilingCategoryPair::NETWORK, - MakeUnique( + NetworkMarkerPayload( static_cast(aChannelId), PromiseFlatCString(spec).get(), aType, aStart, aEnd, aPriority, aCount, aCacheDisposition, aTimings, PromiseFlatCString(redirect_spec).get(), std::move(aSource))); @@ -4211,9 +4206,9 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName, } AUTO_PROFILER_STATS(add_marker_with_TracingMarkerPayload); - auto payload = MakeUnique( - aCategoryString, aKind, aDocShellId, aDocShellHistoryId); - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload)); + profiler_add_marker(aMarkerName, aCategoryPair, + TracingMarkerPayload(aCategoryString, aKind, aDocShellId, + aDocShellHistoryId)); } void profiler_tracing(const char* aCategoryString, const char* aMarkerName, @@ -4231,10 +4226,10 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName, } AUTO_PROFILER_STATS(add_marker_with_TracingMarkerPayload); - auto payload = - MakeUnique(aCategoryString, aKind, aDocShellId, - aDocShellHistoryId, std::move(aCause)); - racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload)); + profiler_add_marker( + aMarkerName, aCategoryPair, + TracingMarkerPayload(aCategoryString, aKind, aDocShellId, + aDocShellHistoryId, std::move(aCause))); } void profiler_add_text_marker( @@ -4247,8 +4242,8 @@ void profiler_add_text_marker( AUTO_PROFILER_STATS(add_marker_with_TextMarkerPayload); profiler_add_marker( aMarkerName, aCategoryPair, - MakeUnique(aText, aStartTime, aEndTime, aDocShellId, - aDocShellHistoryId, std::move(aCause))); + TextMarkerPayload(aText, aStartTime, aEndTime, aDocShellId, + aDocShellHistoryId, std::move(aCause))); } void profiler_set_js_context(JSContext* aCx) { diff --git a/tools/profiler/public/GeckoProfiler.h b/tools/profiler/public/GeckoProfiler.h index 31bf1c4c3211..4870870185ff 100644 --- a/tools/profiler/public/GeckoProfiler.h +++ b/tools/profiler/public/GeckoProfiler.h @@ -691,18 +691,19 @@ void profiler_add_marker(const char* aMarkerName, // the argument list used to construct that `PayloadType`. E.g.: // `PROFILER_ADD_MARKER_WITH_PAYLOAD("Load", DOM, TextMarkerPayload, // ("text", start, end, ds, dsh))` -# define PROFILER_ADD_MARKER_WITH_PAYLOAD( \ - markerName, categoryPair, PayloadType, parenthesizedPayloadArgs) \ - do { \ - AUTO_PROFILER_STATS(add_marker_with_##PayloadType); \ - ::profiler_add_marker( \ - markerName, ::JS::ProfilingCategoryPair::categoryPair, \ - ::mozilla::MakeUnique parenthesizedPayloadArgs); \ +# define PROFILER_ADD_MARKER_WITH_PAYLOAD( \ + markerName, categoryPair, PayloadType, parenthesizedPayloadArgs) \ + do { \ + AUTO_PROFILER_STATS(add_marker_with_##PayloadType); \ + ::profiler_add_marker(markerName, \ + ::JS::ProfilingCategoryPair::categoryPair, \ + PayloadType parenthesizedPayloadArgs); \ } while (false) void profiler_add_marker(const char* aMarkerName, JS::ProfilingCategoryPair aCategoryPair, - mozilla::UniquePtr aPayload); + const ProfilerMarkerPayload& aPayload); + void profiler_add_js_marker(const char* aMarkerName); void profiler_add_js_allocation_marker(JS::RecordAllocationInfo&& info); diff --git a/tools/profiler/public/ProfilerMarkerPayload.h b/tools/profiler/public/ProfilerMarkerPayload.h index 2fa62834fc31..9b05a6e2cf20 100644 --- a/tools/profiler/public/ProfilerMarkerPayload.h +++ b/tools/profiler/public/ProfilerMarkerPayload.h @@ -651,7 +651,7 @@ class LogMarkerPayload : public ProfilerMarkerPayload { class JsAllocationMarkerPayload : public ProfilerMarkerPayload { public: JsAllocationMarkerPayload(const mozilla::TimeStamp& aStartTime, - const JS::RecordAllocationInfo& aInfo, + JS::RecordAllocationInfo&& aInfo, UniqueProfilerBacktrace aStack) : ProfilerMarkerPayload(aStartTime, aStartTime, mozilla::Nothing(), mozilla::Nothing(), std::move(aStack)), @@ -699,6 +699,19 @@ class JsAllocationMarkerPayload : public ProfilerMarkerPayload { namespace mozilla { +// Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no +// payloads. +template <> +struct BlocksRingBuffer::Serializer { + static Length Bytes(const ProfilerMarkerPayload* aPayload) { + return ProfilerMarkerPayload::TagAndSerializationBytes(aPayload); + } + + static void Write(EntryWriter& aEW, const ProfilerMarkerPayload* aPayload) { + ProfilerMarkerPayload::TagAndSerialize(aPayload, aEW); + } +}; + // Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no // payloads. template <>