From 79c0e0c0ef2ea0cc1726c0b9ab007476ce51a147 Mon Sep 17 00:00:00 2001 From: Gerald Squelart Date: Thu, 1 Oct 2020 11:02:54 +0000 Subject: [PATCH] Bug 1667915 - Marker type is now given as a reified empty argument instead of a template argument - r=gregtatum This makes it clearer where marker-type-specific payload arguments start, just after the marker type object. Also improved the main API documentation. Differential Revision: https://phabricator.services.mozilla.com/D91681 --- dom/events/EventDispatcher.cpp | 15 +- .../baseprofiler/public/BaseProfilerMarkers.h | 79 ++++--- mozglue/tests/TestBaseProfiler.cpp | 200 ++++++++---------- tools/profiler/public/ProfilerMarkers.h | 72 ++++--- tools/profiler/tests/gtest/GeckoProfiler.cpp | 77 ++++--- 5 files changed, 240 insertions(+), 203 deletions(-) diff --git a/dom/events/EventDispatcher.cpp b/dom/events/EventDispatcher.cpp index 097dd7d1895c..f7a6c2ddb0eb 100644 --- a/dom/events/EventDispatcher.cpp +++ b/dom/events/EventDispatcher.cpp @@ -1063,19 +1063,18 @@ nsresult EventDispatcher::Dispatch(nsISupports* aTarget, } }; - profiler_add_marker( - "DOMEvent", geckoprofiler::category::DOM, - {MarkerTiming::IntervalStart(), - MarkerInnerWindowId(innerWindowId)}, - typeStr, aEvent->mTimeStamp); + profiler_add_marker("DOMEvent", geckoprofiler::category::DOM, + {MarkerTiming::IntervalStart(), + MarkerInnerWindowId(innerWindowId)}, + DOMEventMarker{}, typeStr, aEvent->mTimeStamp); EventTargetChainItem::HandleEventTargetChain(chain, postVisitor, aCallback, cd); - profiler_add_marker( + profiler_add_marker( "DOMEvent", geckoprofiler::category::DOM, - {MarkerTiming::IntervalEnd(), std::move(innerWindowId)}, typeStr, - aEvent->mTimeStamp); + {MarkerTiming::IntervalEnd(), std::move(innerWindowId)}, + DOMEventMarker{}, typeStr, aEvent->mTimeStamp); } else #endif { diff --git a/mozglue/baseprofiler/public/BaseProfilerMarkers.h b/mozglue/baseprofiler/public/BaseProfilerMarkers.h index 9842e3755292..46e201b9e20a 100644 --- a/mozglue/baseprofiler/public/BaseProfilerMarkers.h +++ b/mozglue/baseprofiler/public/BaseProfilerMarkers.h @@ -21,11 +21,11 @@ // supporting types. // // To then record markers: -// - Use `baseprofiler::AddMarker(...)` from -// mozglue or other libraries that are outside of xul, especially if they may -// happen outside of xpcom's lifetime (typically startup, shutdown, or tests). +// - Use `baseprofiler::AddMarker(...)` from mozglue or other libraries that +// are outside of xul, especially if they may happen outside of xpcom's +// lifetime (typically startup, shutdown, or tests). // - Otherwise #include "ProfilerMarkers.h" instead, and use -// `profiler_add_marker(...)`. +// `profiler_add_marker(...)`. // See these functions for more details. #ifndef BaseProfilerMarkers_h @@ -49,6 +49,7 @@ # include "mozilla/ProfileChunkedBuffer.h" # include "mozilla/TimeStamp.h" +# include "mozilla/Unused.h" # include # include @@ -56,42 +57,56 @@ namespace mozilla::baseprofiler { -template -ProfileBufferBlockIndex AddMarkerToBuffer(ProfileChunkedBuffer& aBuffer, - const ProfilerString8View& aName, - const MarkerCategory& aCategory, - MarkerOptions&& aOptions, - const Ts&... aTs) { +// Add a marker to a given buffer. `AddMarker()` and related macros should be +// used in most cases, see below for more information about them and the +// parameters; This function may be useful when markers need to be recorded in a +// local buffer outside of the main profiler buffer. +template +ProfileBufferBlockIndex AddMarkerToBuffer( + ProfileChunkedBuffer& aBuffer, const ProfilerString8View& aName, + const MarkerCategory& aCategory, MarkerOptions&& aOptions, + MarkerType aMarkerType, const PayloadArguments&... aPayloadArguments) { + Unused << aMarkerType; // Only the empty object type is useful. return base_profiler_markers_detail::AddMarkerToBuffer( aBuffer, aName, aCategory, std::move(aOptions), - ::mozilla::baseprofiler::profiler_capture_backtrace_into, aTs...); + ::mozilla::baseprofiler::profiler_capture_backtrace_into, + aPayloadArguments...); } // Add a marker (without payload) to a given buffer. inline ProfileBufferBlockIndex AddMarkerToBuffer( ProfileChunkedBuffer& aBuffer, const ProfilerString8View& aName, const MarkerCategory& aCategory, MarkerOptions&& aOptions = {}) { - return AddMarkerToBuffer(aBuffer, aName, aCategory, - std::move(aOptions)); + return AddMarkerToBuffer(aBuffer, aName, aCategory, std::move(aOptions), + markers::NoPayload{}); } -template -ProfileBufferBlockIndex AddMarker(const ProfilerString8View& aName, - const MarkerCategory& aCategory, - MarkerOptions&& aOptions, const Ts&... aTs) { +// Add a marker to the Base Profiler buffer. +// - aName: Main name of this marker. +// - aCategory: Category for this marker. +// - aOptions: Optional settings (such as timing, inner window id, +// backtrace...), see `MarkerOptions` for details. +// - aMarkerType: Empty object that specifies the type of marker. +// - aPayloadArguments: Arguments expected by this marker type's +// ` StreamJSONMarkerData` function. +template +ProfileBufferBlockIndex AddMarker( + const ProfilerString8View& aName, const MarkerCategory& aCategory, + MarkerOptions&& aOptions, MarkerType aMarkerType, + const PayloadArguments&... aPayloadArguments) { if (!baseprofiler::profiler_can_accept_markers()) { return {}; } - return ::mozilla::baseprofiler::AddMarkerToBuffer( + return ::mozilla::baseprofiler::AddMarkerToBuffer( base_profiler_markers_detail::CachedBaseCoreBuffer(), aName, aCategory, - std::move(aOptions), aTs...); + std::move(aOptions), aMarkerType, aPayloadArguments...); } // Add a marker (without payload) to the Base Profiler buffer. inline ProfileBufferBlockIndex AddMarker(const ProfilerString8View& aName, const MarkerCategory& aCategory, MarkerOptions&& aOptions = {}) { - return AddMarker(aName, aCategory, std::move(aOptions)); + return AddMarker(aName, aCategory, std::move(aOptions), markers::NoPayload{}); } // Marker types' StreamJSONMarkerData functions should use this to correctly @@ -107,6 +122,8 @@ inline void WritePropertyTime(JSONWriter& aWriter, } // namespace mozilla::baseprofiler +// Same as `AddMarker()` (without payload). This macro is safe to use even if +// MOZ_GECKO_PROFILER is not #defined. # define BASE_PROFILER_MARKER_UNTYPED(markerName, categoryName, ...) \ do { \ AUTO_PROFILER_STATS(BASE_PROFILER_MARKER_UNTYPED); \ @@ -115,14 +132,16 @@ inline void WritePropertyTime(JSONWriter& aWriter, ##__VA_ARGS__); \ } while (false) +// Same as `AddMarker()` (with payload). This macro is safe to use even if +// MOZ_GECKO_PROFILER is not #defined. # define BASE_PROFILER_MARKER(markerName, categoryName, options, MarkerType, \ ...) \ do { \ AUTO_PROFILER_STATS(BASE_PROFILER_MARKER_with_##MarkerType); \ - ::mozilla::baseprofiler::AddMarker< \ - ::mozilla::baseprofiler::markers::MarkerType>( \ + ::mozilla::baseprofiler::AddMarker( \ markerName, ::mozilla::baseprofiler::category::categoryName, \ - options, ##__VA_ARGS__); \ + options, ::mozilla::baseprofiler::markers::MarkerType{}, \ + ##__VA_ARGS__); \ } while (false) namespace mozilla::baseprofiler::markers { @@ -138,13 +157,14 @@ struct Text { }; } // namespace mozilla::baseprofiler::markers +// Add a text marker. This macro is safe to use even if MOZ_GECKO_PROFILER is +// not #defined. # define BASE_PROFILER_MARKER_TEXT(markerName, categoryName, options, text) \ do { \ AUTO_PROFILER_STATS(BASE_PROFILER_MARKER_TEXT); \ - ::mozilla::baseprofiler::AddMarker< \ - ::mozilla::baseprofiler::markers::Text>( \ + ::mozilla::baseprofiler::AddMarker( \ markerName, ::mozilla::baseprofiler::category::categoryName, \ - options, text); \ + options, ::mozilla::baseprofiler::markers::Text{}, text); \ } while (false) namespace mozilla::baseprofiler { @@ -171,9 +191,8 @@ class MOZ_RAII AutoProfilerTextMarker { ~AutoProfilerTextMarker() { mOptions.TimingRef().SetIntervalEnd(); AUTO_PROFILER_STATS(AUTO_BASE_PROFILER_MARKER_TEXT); - AddMarker( - ProfilerString8View::WrapNullTerminatedString(mMarkerName), mCategory, - std::move(mOptions), mText); + AddMarker(ProfilerString8View::WrapNullTerminatedString(mMarkerName), + mCategory, std::move(mOptions), markers::Text{}, mText); } protected: @@ -185,6 +204,8 @@ class MOZ_RAII AutoProfilerTextMarker { } // namespace mozilla::baseprofiler +// Creates an AutoProfilerTextMarker RAII object. This macro is safe to use +// even if MOZ_GECKO_PROFILER is not #defined. # define AUTO_BASE_PROFILER_MARKER_TEXT(markerName, categoryName, options, \ text) \ ::mozilla::baseprofiler::AutoProfilerTextMarker BASE_PROFILER_RAII( \ diff --git a/mozglue/tests/TestBaseProfiler.cpp b/mozglue/tests/TestBaseProfiler.cpp index 535b35872fcb..6c8999f03f98 100644 --- a/mozglue/tests/TestBaseProfiler.cpp +++ b/mozglue/tests/TestBaseProfiler.cpp @@ -3449,16 +3449,18 @@ void TestProfiler() { baseprofiler::FileIOMarkerPayload( "operation", "source", "filename", TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed(), std::move(cause))); - baseprofiler::AddMarker( - "m2fileio", mozilla::baseprofiler::category::OTHER, {}, "op2", "src2", - "f2", MarkerThreadId{}); - baseprofiler::AddMarker( + baseprofiler::AddMarker("m2fileio", mozilla::baseprofiler::category::OTHER, + {}, mozilla::baseprofiler::markers::FileIO{}, "op2", + "src2", "f2", MarkerThreadId{}); + baseprofiler::AddMarker( "m2fileio-capture", mozilla::baseprofiler::category::OTHER, - MarkerStack::Capture(), "op2", "src2", "f2", MarkerThreadId{}); - baseprofiler::AddMarker( + MarkerStack::Capture(), mozilla::baseprofiler::markers::FileIO{}, "op2", + "src2", "f2", MarkerThreadId{}); + baseprofiler::AddMarker( "m2fileio-take-backtrace", mozilla::baseprofiler::category::OTHER, MarkerStack::TakeBacktrace(baseprofiler::profiler_capture_backtrace()), - "op2", "src2", "f2", MarkerThreadId{}); + mozilla::baseprofiler::markers::FileIO{}, "op2", "src2", "f2", + MarkerThreadId{}); baseprofiler::profiler_add_marker( "UserTimingMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER, @@ -3545,50 +3547,48 @@ void TestProfiler() { "default-templated markers 2.0 with option", mozilla::baseprofiler::category::OTHER, MarkerInnerWindowId(123))); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker<::mozilla::baseprofiler::markers::NoPayload>( - "explicitly-default-templated markers 2.0 without options", - mozilla::baseprofiler::category::OTHER, {})); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "explicitly-default-templated markers 2.0 without options", + mozilla::baseprofiler::category::OTHER, {}, + ::mozilla::baseprofiler::markers::NoPayload{})); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker<::mozilla::baseprofiler::markers::NoPayload>( - "explicitly-default-templated markers 2.0 with option", - mozilla::baseprofiler::category::OTHER, MarkerInnerWindowId(123))); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "explicitly-default-templated markers 2.0 with option", + mozilla::baseprofiler::category::OTHER, MarkerInnerWindowId(123), + ::mozilla::baseprofiler::markers::NoPayload{})); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "tracing", mozilla::baseprofiler::category::OTHER, {}, "category")); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "tracing", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::Tracing{}, "category")); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "mark", mozilla::baseprofiler::category::OTHER, {}, "mark name")); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "mark", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::UserTimingMark{}, "mark name")); - MOZ_RELEASE_ASSERT(baseprofiler::AddMarker< - mozilla::baseprofiler::markers::UserTimingMeasure>( - "measure", mozilla::baseprofiler::category::OTHER, {}, "measure name", + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "measure", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::UserTimingMeasure{}, "measure name", Some(ProfilerString8View("start")), Some(ProfilerString8View("end")))); MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "hang", mozilla::baseprofiler::category::OTHER, {})); + baseprofiler::AddMarker("hang", mozilla::baseprofiler::category::OTHER, + {}, mozilla::baseprofiler::markers::Hang{})); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "longtask", mozilla::baseprofiler::category::OTHER, {})); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "longtask", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::LongTask{})); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "text", mozilla::baseprofiler::category::OTHER, {}, "text text")); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "text", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::Text{}, "text text")); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "log", mozilla::baseprofiler::category::OTHER, {}, "module", - "text")); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "log", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::Log{}, "module", "text")); - MOZ_RELEASE_ASSERT( - baseprofiler::AddMarker( - "media sample", mozilla::baseprofiler::category::OTHER, {}, 123, - 456)); + MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( + "media sample", mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::MediaSample{}, 123, 456)); printf("Sleep 1s...\n"); { @@ -3797,62 +3797,55 @@ void TestUserMarker() { mozilla::ProfileChunkedBuffer buffer( mozilla::ProfileChunkedBuffer::ThreadSafety::WithoutMutex, chunkManager); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, {}, - std::string("payload text"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, {}, + MarkerTypeTestMinimal{}, std::string("payload text"))); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerThreadId(123), std::string("ThreadId(123)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerThreadId(123), MarkerTypeTestMinimal{}, + std::string("ThreadId(123)"))); auto start = mozilla::TimeStamp::NowUnfuzzed(); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerTiming::InstantAt(start), - std::string("InstantAt(start)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerTiming::InstantAt(start), MarkerTypeTestMinimal{}, + std::string("InstantAt(start)"))); auto then = mozilla::TimeStamp::NowUnfuzzed(); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerTiming::IntervalStart(start), - std::string("IntervalStart(start)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerTiming::IntervalStart(start), MarkerTypeTestMinimal{}, + std::string("IntervalStart(start)"))); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerTiming::IntervalEnd(then), - std::string("IntervalEnd(then)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerTiming::IntervalEnd(then), MarkerTypeTestMinimal{}, + std::string("IntervalEnd(then)"))); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerTiming::Interval(start, then), - std::string("Interval(start, then)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerTiming::Interval(start, then), MarkerTypeTestMinimal{}, + std::string("Interval(start, then)"))); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerTiming::IntervalUntilNowFrom(start), - std::string("IntervalUntilNowFrom(start)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerTiming::IntervalUntilNowFrom(start), + MarkerTypeTestMinimal{}, std::string("IntervalUntilNowFrom(start)"))); - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerStack::NoStack(), std::string("NoStack"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerStack::NoStack(), MarkerTypeTestMinimal{}, + std::string("NoStack"))); // Note: We cannot test stack-capture here, because the profiler is not // initialized. - MOZ_RELEASE_ASSERT( - mozilla::baseprofiler::AddMarkerToBuffer( - buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, - mozilla::MarkerInnerWindowId(123), - std::string("InnerWindowId(123)"))); + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( + buffer, "test2", mozilla::baseprofiler::category::OTHER_Profiling, + mozilla::MarkerInnerWindowId(123), MarkerTypeTestMinimal{}, + std::string("InnerWindowId(123)"))); # ifdef DEBUG buffer.Dump(); @@ -3882,47 +3875,42 @@ void TestPredefinedMarkers() { mozilla::ProfileChunkedBuffer buffer( mozilla::ProfileChunkedBuffer::ThreadSafety::WithoutMutex, chunkManager); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::Tracing>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("tracing"), - mozilla::baseprofiler::category::OTHER, {}, "category")); + mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::Tracing{}, "category")); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::UserTimingMark>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("mark"), mozilla::baseprofiler::category::OTHER, - {}, "mark name")); + {}, mozilla::baseprofiler::markers::UserTimingMark{}, "mark name")); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::UserTimingMeasure>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("measure"), - mozilla::baseprofiler::category::OTHER, {}, "measure name ", + mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::UserTimingMeasure{}, "measure name ", mozilla::Some(mozilla::ProfilerString8View(" start ")), mozilla::Some(mozilla::ProfilerString8View("end")))); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::Hang>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("hang"), mozilla::baseprofiler::category::OTHER, - {})); + {}, mozilla::baseprofiler::markers::Hang{})); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::LongTask>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("long task"), - mozilla::baseprofiler::category::OTHER, {})); + mozilla::baseprofiler::category::OTHER, {}, + mozilla::baseprofiler::markers::LongTask{})); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::Text>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("text"), mozilla::baseprofiler::category::OTHER, - {}, "text text")); + {}, mozilla::baseprofiler::markers::Text{}, "text text")); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::Log>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("log"), mozilla::baseprofiler::category::OTHER, - {}, "module", "text")); + {}, mozilla::baseprofiler::markers::Log{}, "module", "text")); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer< - mozilla::baseprofiler::markers::MediaSample>( + MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("media"), mozilla::baseprofiler::category::OTHER, - {}, 123, 456)); + {}, mozilla::baseprofiler::markers::MediaSample{}, 123, 456)); # ifdef DEBUG buffer.Dump(); diff --git a/tools/profiler/public/ProfilerMarkers.h b/tools/profiler/public/ProfilerMarkers.h index a5bd2b98f28c..353ed8f20f41 100644 --- a/tools/profiler/public/ProfilerMarkers.h +++ b/tools/profiler/public/ProfilerMarkers.h @@ -22,11 +22,11 @@ // types. // // To then record markers: -// - Use `baseprofiler::AddMarker(...)` from -// mozglue or other libraries that are outside of xul, especially if they may -// happen outside of xpcom's lifetime (typically startup, shutdown, or tests). +// - Use `baseprofiler::AddMarker(...)` from mozglue or other libraries that are +// outside of xul, especially if they may happen outside of xpcom's lifetime +// (typically startup, shutdown, or tests). // - Otherwise #include "ProfilerMarkers.h" instead, and use -// `profiler_add_marker(...)`. +// `profiler_add_marker(...)`. // See these functions for more details. #ifndef ProfilerMarkers_h @@ -53,15 +53,20 @@ namespace geckoprofiler::category { using namespace ::mozilla::baseprofiler::category; } -template +// Add a marker to a given buffer. `AddMarker()` and related macros should be +// used in most cases, see below for more information about them and the +// paramters; This function may be useful when markers need to be recorded in a +// local buffer outside of the main profiler buffer. +template mozilla::ProfileBufferBlockIndex AddMarkerToBuffer( mozilla::ProfileChunkedBuffer& aBuffer, const mozilla::ProfilerString8View& aName, const mozilla::MarkerCategory& aCategory, mozilla::MarkerOptions&& aOptions, - const Ts&... aTs) { + MarkerType aMarkerType, const PayloadArguments&... aPayloadArguments) { + mozilla::Unused << aMarkerType; // Only the empty object type is useful. return mozilla::base_profiler_markers_detail::AddMarkerToBuffer( aBuffer, aName, aCategory, std::move(aOptions), - ::profiler_capture_backtrace_into, aTs...); + ::profiler_capture_backtrace_into, aPayloadArguments...); } // Add a marker (without payload) to a given buffer. @@ -70,21 +75,29 @@ inline mozilla::ProfileBufferBlockIndex AddMarkerToBuffer( const mozilla::ProfilerString8View& aName, const mozilla::MarkerCategory& aCategory, mozilla::MarkerOptions&& aOptions = {}) { - return AddMarkerToBuffer( - aBuffer, aName, aCategory, std::move(aOptions)); + return AddMarkerToBuffer(aBuffer, aName, aCategory, std::move(aOptions), + mozilla::baseprofiler::markers::NoPayload{}); } -template +// Add a marker to the Gecko Profiler buffer. +// - aName: Main name of this marker. +// - aCategory: Category for this marker. +// - aOptions: Optional settings (such as timing, inner window id, +// backtrace...), see `MarkerOptions` for details. +// - aMarkerType: Empty object that specifies the type of marker. +// - aPayloadArguments: Arguments expected by this marker type's +// ` StreamJSONMarkerData` function. +template mozilla::ProfileBufferBlockIndex profiler_add_marker( const mozilla::ProfilerString8View& aName, const mozilla::MarkerCategory& aCategory, mozilla::MarkerOptions&& aOptions, - const Ts&... aTs) { + MarkerType aMarkerType, const PayloadArguments&... aPayloadArguments) { if (!profiler_can_accept_markers()) { return {}; } - return ::AddMarkerToBuffer( - profiler_markers_detail::CachedCoreBuffer(), aName, aCategory, - std::move(aOptions), aTs...); + return ::AddMarkerToBuffer(profiler_markers_detail::CachedCoreBuffer(), aName, + aCategory, std::move(aOptions), aMarkerType, + aPayloadArguments...); } // Add a marker (without payload) to the Gecko Profiler buffer. @@ -92,10 +105,12 @@ inline mozilla::ProfileBufferBlockIndex profiler_add_marker( const mozilla::ProfilerString8View& aName, const mozilla::MarkerCategory& aCategory, mozilla::MarkerOptions&& aOptions = {}) { - return profiler_add_marker( - aName, aCategory, std::move(aOptions)); + return profiler_add_marker(aName, aCategory, std::move(aOptions), + mozilla::baseprofiler::markers::NoPayload{}); } +// Same as `profiler_add_marker()` (without payload). This macro is safe to use +// even if MOZ_GECKO_PROFILER is not #defined. # define PROFILER_MARKER_UNTYPED(markerName, categoryName, ...) \ do { \ AUTO_PROFILER_STATS(PROFILER_MARKER_UNTYPED); \ @@ -103,12 +118,14 @@ inline mozilla::ProfileBufferBlockIndex profiler_add_marker( markerName, ::geckoprofiler::category::categoryName, ##__VA_ARGS__); \ } while (false) +// Same as `profiler_add_marker()` (with payload). This macro is safe to use +// even if MOZ_GECKO_PROFILER is not #defined. # define PROFILER_MARKER(markerName, categoryName, options, MarkerType, ...) \ do { \ AUTO_PROFILER_STATS(PROFILER_MARKER_with_##MarkerType); \ - ::profiler_add_marker<::geckoprofiler::markers::MarkerType>( \ + ::profiler_add_marker( \ markerName, ::geckoprofiler::category::categoryName, options, \ - ##__VA_ARGS__); \ + ::geckoprofiler::markers::MarkerType{}, ##__VA_ARGS__); \ } while (false) namespace geckoprofiler::markers { @@ -116,11 +133,14 @@ namespace geckoprofiler::markers { using Text = ::mozilla::baseprofiler::markers::Text; } // namespace geckoprofiler::markers -# define PROFILER_MARKER_TEXT(markerName, categoryName, options, text) \ - do { \ - AUTO_PROFILER_STATS(PROFILER_MARKER_TEXT); \ - ::profiler_add_marker<::geckoprofiler::markers::Text>( \ - markerName, ::geckoprofiler::category::categoryName, options, text); \ +// Add a text marker. This macro is safe to use even if MOZ_GECKO_PROFILER is +// not #defined. +# define PROFILER_MARKER_TEXT(markerName, categoryName, options, text) \ + do { \ + AUTO_PROFILER_STATS(PROFILER_MARKER_TEXT); \ + ::profiler_add_marker(markerName, \ + ::geckoprofiler::category::categoryName, options, \ + ::geckoprofiler::markers::Text{}, text); \ } while (false) // RAII object that adds a PROFILER_MARKER_TEXT when destroyed; the marker's @@ -146,9 +166,9 @@ class MOZ_RAII AutoProfilerTextMarker { ~AutoProfilerTextMarker() { mOptions.TimingRef().SetIntervalEnd(); AUTO_PROFILER_STATS(AUTO_PROFILER_MARKER_TEXT); - profiler_add_marker( + profiler_add_marker( mozilla::ProfilerString8View::WrapNullTerminatedString(mMarkerName), - mCategory, std::move(mOptions), mText); + mCategory, std::move(mOptions), geckoprofiler::markers::Text{}, mText); } protected: @@ -158,6 +178,8 @@ class MOZ_RAII AutoProfilerTextMarker { nsCString mText; }; +// Creates an AutoProfilerTextMarker RAII object. This macro is safe to use +// even if MOZ_GECKO_PROFILER is not #defined. # define AUTO_PROFILER_MARKER_TEXT(markerName, categoryName, options, text) \ AutoProfilerTextMarker PROFILER_RAII( \ markerName, ::mozilla::baseprofiler::category::categoryName, options, \ diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp index ad329ff0f6f6..fc3044c66f38 100644 --- a/tools/profiler/tests/gtest/GeckoProfiler.cpp +++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp @@ -714,9 +714,10 @@ TEST(GeckoProfiler, Markers) PROFILER_MARKER("explicitly-default-templated markers 2.0 with empty options", OTHER, {}, NoPayload); - MOZ_RELEASE_ASSERT(profiler_add_marker<::geckoprofiler::markers::NoPayload>( + MOZ_RELEASE_ASSERT(profiler_add_marker( "explicitly-default-templated markers 2.0 with option", - geckoprofiler::category::OTHER, {})); + geckoprofiler::category::OTHER, {}, + ::geckoprofiler::markers::NoPayload{})); // Used in markers below. TimeStamp ts1 = TimeStamp::NowUnfuzzed(); @@ -738,19 +739,19 @@ TEST(GeckoProfiler, Markers) "FileIOMarkerPayload marker", OTHER, FileIOMarkerPayload, ("operation", "source", "filename", ts1, ts2, nullptr)); - MOZ_RELEASE_ASSERT(profiler_add_marker( + MOZ_RELEASE_ASSERT(profiler_add_marker( "FileIOMarkerPayload marker 2.0", geckoprofiler::category::OTHER, - MarkerTiming::Interval(ts1, ts2), "operation", "source", "filename", - MarkerThreadId{})); + MarkerTiming::Interval(ts1, ts2), geckoprofiler::markers::FileIO{}, + "operation", "source", "filename", MarkerThreadId{})); PROFILER_ADD_MARKER_WITH_PAYLOAD( "FileIOMarkerPayload marker off-MT", OTHER, FileIOMarkerPayload, ("operation2", "source2", "filename2", ts1, ts2, nullptr, Some(123))); - MOZ_RELEASE_ASSERT(profiler_add_marker( + MOZ_RELEASE_ASSERT(profiler_add_marker( "FileIOMarkerPayload marker 2.0 off-MT", geckoprofiler::category::OTHER, - MarkerTiming::Interval(ts1, ts2), "operation2", "source2", "filename2", - MarkerThreadId{123})); + MarkerTiming::Interval(ts1, ts2), geckoprofiler::markers::FileIO{}, + "operation2", "source2", "filename2", MarkerThreadId{123})); // Other markers in alphabetical order of payload class names. @@ -856,36 +857,42 @@ TEST(GeckoProfiler, Markers) mozilla::ipc::MessageDirection::eSending, mozilla::ipc::MessagePhase::Endpoint, false, ts1)); - MOZ_RELEASE_ASSERT(profiler_add_marker( - "Tracing", geckoprofiler::category::OTHER, {}, "category")); + MOZ_RELEASE_ASSERT( + profiler_add_marker("Tracing", geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::Tracing{}, "category")); + + MOZ_RELEASE_ASSERT(profiler_add_marker( + "UserTimingMark", geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::UserTimingMark{}, "mark name")); + + MOZ_RELEASE_ASSERT(profiler_add_marker( + "UserTimingMeasure", geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::UserTimingMeasure{}, "measure name", + Some(mozilla::ProfilerString8View("start")), + Some(mozilla::ProfilerString8View("end")))); + + MOZ_RELEASE_ASSERT(profiler_add_marker("Hang", geckoprofiler::category::OTHER, + {}, geckoprofiler::markers::Hang{})); + + MOZ_RELEASE_ASSERT(profiler_add_marker("LongTask", + geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::LongTask{})); + + MOZ_RELEASE_ASSERT(profiler_add_marker("Text", geckoprofiler::category::OTHER, + {}, geckoprofiler::markers::Text{}, + "Text text")); + + MOZ_RELEASE_ASSERT(profiler_add_marker("Log", geckoprofiler::category::OTHER, + {}, geckoprofiler::markers::Log{}, + "module", "log text")); MOZ_RELEASE_ASSERT( - profiler_add_marker( - "UserTimingMark", geckoprofiler::category::OTHER, {}, "mark name")); + profiler_add_marker("MediaSample", geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::MediaSample{}, 123, 456)); - MOZ_RELEASE_ASSERT( - profiler_add_marker( - "UserTimingMeasure", geckoprofiler::category::OTHER, {}, - "measure name", Some(mozilla::ProfilerString8View("start")), - Some(mozilla::ProfilerString8View("end")))); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "Hang", geckoprofiler::category::OTHER, {})); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "LongTask", geckoprofiler::category::OTHER, {})); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "Text", geckoprofiler::category::OTHER, {}, "Text text")); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "Log", geckoprofiler::category::OTHER, {}, "module", "log text")); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "MediaSample", geckoprofiler::category::OTHER, {}, 123, 456)); - - MOZ_RELEASE_ASSERT(profiler_add_marker( - "Budget", geckoprofiler::category::OTHER, {})); + MOZ_RELEASE_ASSERT(profiler_add_marker("Budget", + geckoprofiler::category::OTHER, {}, + geckoprofiler::markers::Budget{})); SpliceableChunkedJSONWriter w; w.Start();