From 46fed8e10fc2f0a0518c24d809b684853a1a3f6e Mon Sep 17 00:00:00 2001 From: Gerald Squelart Date: Wed, 18 Nov 2020 21:46:56 +0000 Subject: [PATCH] Bug 1675409 - Migrated LongTaskMarkerPayload to Markers 2.0 API - r=gregtatum Differential Revision: https://phabricator.services.mozilla.com/D96038 --- .../public/BaseProfilerMarkerTypes.h | 15 ---------- mozglue/tests/TestBaseProfiler.cpp | 11 -------- tools/profiler/public/ProfilerMarkerTypes.h | 1 - tools/profiler/tests/gtest/GeckoProfiler.cpp | 18 ------------ xpcom/threads/nsThread.cpp | 28 +++++++++++++++---- 5 files changed, 22 insertions(+), 51 deletions(-) diff --git a/mozglue/baseprofiler/public/BaseProfilerMarkerTypes.h b/mozglue/baseprofiler/public/BaseProfilerMarkerTypes.h index 3f8ab5980f1d..6ffdde2be155 100644 --- a/mozglue/baseprofiler/public/BaseProfilerMarkerTypes.h +++ b/mozglue/baseprofiler/public/BaseProfilerMarkerTypes.h @@ -111,21 +111,6 @@ struct UserTimingMeasure { } }; -struct LongTask { - static constexpr Span MarkerTypeName() { - return MakeStringSpan("MainThreadLongTask"); - } - static void StreamJSONMarkerData(SpliceableJSONWriter& aWriter) { - aWriter.StringProperty("category", "LongTask"); - } - static MarkerSchema MarkerTypeDisplay() { - using MS = MarkerSchema; - MS schema{MS::Location::markerChart, MS::Location::markerTable}; - schema.AddKeyLabelFormat("category", "Type", MS::Format::string); - return schema; - } -}; - struct Log { static constexpr Span MarkerTypeName() { return MakeStringSpan("Log"); diff --git a/mozglue/tests/TestBaseProfiler.cpp b/mozglue/tests/TestBaseProfiler.cpp index 736d021f0d8d..ccf0a4062570 100644 --- a/mozglue/tests/TestBaseProfiler.cpp +++ b/mozglue/tests/TestBaseProfiler.cpp @@ -3448,10 +3448,6 @@ void TestProfiler() { mozilla::baseprofiler::markers::UserTimingMeasure{}, "measure name", Some(ProfilerString8View("start")), Some(ProfilerString8View("end")))); - MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( - "longtask", mozilla::baseprofiler::category::OTHER, {}, - mozilla::baseprofiler::markers::LongTask{})); - MOZ_RELEASE_ASSERT(baseprofiler::AddMarker( "text", mozilla::baseprofiler::category::OTHER, {}, mozilla::baseprofiler::markers::Text{}, "text text")); @@ -3524,8 +3520,6 @@ void TestProfiler() { svnpos); MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"Log\",") != svnpos); MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"MediaSample\",") != svnpos); - MOZ_RELEASE_ASSERT(profileSV.find("\"name\": \"MainThreadLongTask\",") != - svnpos); MOZ_RELEASE_ASSERT(profileSV.find("\"display\": [") != svnpos); MOZ_RELEASE_ASSERT(profileSV.find("\"marker-chart\"") != svnpos); MOZ_RELEASE_ASSERT(profileSV.find("\"marker-table\"") != svnpos); @@ -4039,11 +4033,6 @@ void TestPredefinedMarkers() { mozilla::Some(mozilla::ProfilerString8View(" start ")), mozilla::Some(mozilla::ProfilerString8View("end")))); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( - buffer, std::string_view("long task"), - mozilla::baseprofiler::category::OTHER, {}, - mozilla::baseprofiler::markers::LongTask{})); - MOZ_RELEASE_ASSERT(mozilla::baseprofiler::AddMarkerToBuffer( buffer, std::string_view("text"), mozilla::baseprofiler::category::OTHER, {}, mozilla::baseprofiler::markers::Text{}, "text text")); diff --git a/tools/profiler/public/ProfilerMarkerTypes.h b/tools/profiler/public/ProfilerMarkerTypes.h index 2cefeeb72cd8..4fba5fe5d804 100644 --- a/tools/profiler/public/ProfilerMarkerTypes.h +++ b/tools/profiler/public/ProfilerMarkerTypes.h @@ -38,7 +38,6 @@ namespace geckoprofiler::markers { using Tracing = mozilla::baseprofiler::markers::Tracing; using UserTimingMark = mozilla::baseprofiler::markers::UserTimingMark; using UserTimingMeasure = mozilla::baseprofiler::markers::UserTimingMeasure; -using LongTask = mozilla::baseprofiler::markers::LongTask; using Log = mozilla::baseprofiler::markers::Log; using MediaSample = mozilla::baseprofiler::markers::MediaSample; diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp index e77012ca6752..982bffb0a17d 100644 --- a/tools/profiler/tests/gtest/GeckoProfiler.cpp +++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp @@ -802,9 +802,6 @@ TEST(GeckoProfiler, Markers) PROFILER_ADD_MARKER_WITH_PAYLOAD("LogMarkerPayload marker", OTHER, LogMarkerPayload, ("module", "text", ts1)); - PROFILER_ADD_MARKER_WITH_PAYLOAD("LongTaskMarkerPayload marker", OTHER, - LongTaskMarkerPayload, (ts1, ts2)); - PROFILER_ADD_MARKER_WITH_PAYLOAD("NativeAllocationMarkerPayload marker", OTHER, NativeAllocationMarkerPayload, (ts1, 9876543210, 1234, 5678, nullptr)); @@ -907,10 +904,6 @@ TEST(GeckoProfiler, Markers) Some(mozilla::ProfilerString8View("start")), Some(mozilla::ProfilerString8View("end")))); - 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")); @@ -970,7 +963,6 @@ TEST(GeckoProfiler, Markers) S_GCMinorMarkerPayload, S_GCSliceMarkerPayload, S_LogMarkerPayload, - S_LongTaskMarkerPayload, S_NativeAllocationMarkerPayload, S_NetworkMarkerPayload_start, S_NetworkMarkerPayload_stop, @@ -1352,14 +1344,6 @@ TEST(GeckoProfiler, Markers) EXPECT_EQ_JSON(payload["name"], String, "text"); EXPECT_EQ_JSON(payload["module"], String, "module"); - } else if (nameString == "LongTaskMarkerPayload marker") { - EXPECT_EQ(state, S_LongTaskMarkerPayload); - state = State(S_LongTaskMarkerPayload + 1); - EXPECT_EQ(typeString, "MainThreadLongTask"); - EXPECT_TIMING_INTERVAL_AT(ts1Double, ts2Double); - EXPECT_TRUE(payload["stack"].isNull()); - EXPECT_EQ_JSON(payload["category"], String, "LongTask"); - } else if (nameString == "NativeAllocationMarkerPayload marker") { EXPECT_EQ(state, S_NativeAllocationMarkerPayload); state = State(S_NativeAllocationMarkerPayload + 1); @@ -1731,8 +1715,6 @@ TEST(GeckoProfiler, Markers) testedSchemaNames.end()); EXPECT_TRUE(testedSchemaNames.find("UserTimingMeasure") != testedSchemaNames.end()); - EXPECT_TRUE(testedSchemaNames.find("MainThreadLongTask") != - testedSchemaNames.end()); EXPECT_TRUE(testedSchemaNames.find("Log") != testedSchemaNames.end()); EXPECT_TRUE(testedSchemaNames.find("MediaSample") != testedSchemaNames.end()); diff --git a/xpcom/threads/nsThread.cpp b/xpcom/threads/nsThread.cpp index 9c5868af0ce7..409e23ecfbc0 100644 --- a/xpcom/threads/nsThread.cpp +++ b/xpcom/threads/nsThread.cpp @@ -42,9 +42,6 @@ #include "nsThreadSyncDispatch.h" #include "nsServiceManagerUtils.h" #include "GeckoProfiler.h" -#ifdef MOZ_GECKO_PROFILER -# include "ProfilerMarkerPayload.h" -#endif #include "InputEventStatistics.h" #include "ThreadEventQueue.h" #include "ThreadEventTarget.h" @@ -1524,9 +1521,28 @@ void PerformanceCounterState::MaybeReportAccumulatedTime(TimeStamp aNow) { #ifdef MOZ_GECKO_PROFILER if (profiler_thread_is_being_profiled()) { - PROFILER_ADD_MARKER_WITH_PAYLOAD( - mCurrentRunnableIsIdleRunnable ? "LongIdleTask" : "LongTask", OTHER, - LongTaskMarkerPayload, (mCurrentTimeSliceStart, aNow)); + struct LongTaskMarker { + static constexpr Span MarkerTypeName() { + return MakeStringSpan("MainThreadLongTask"); + } + static void StreamJSONMarkerData( + baseprofiler::SpliceableJSONWriter& aWriter) { + aWriter.StringProperty("category", "LongTask"); + } + static MarkerSchema MarkerTypeDisplay() { + using MS = MarkerSchema; + MS schema{MS::Location::markerChart, MS::Location::markerTable}; + schema.AddKeyLabelFormat("category", "Type", MS::Format::string); + return schema; + } + }; + + profiler_add_marker(mCurrentRunnableIsIdleRunnable + ? ProfilerString8View("LongIdleTask") + : ProfilerString8View("LongTask"), + geckoprofiler::category::OTHER, + MarkerTiming::Interval(mCurrentTimeSliceStart, aNow), + LongTaskMarker{}); } #endif }