Backed out 4 changesets (bug 1814908) for causing cppunit failures on TestBaseProfiler CLOSED TREE

Backed out changeset 4fd5e3f3d87b (bug 1814908)
Backed out changeset 6a716b16e8e3 (bug 1814908)
Backed out changeset a8557338bd74 (bug 1814908)
Backed out changeset b1a9a1820b56 (bug 1814908)
This commit is contained in:
Norisz Fay 2023-02-14 14:44:31 +02:00
Родитель c4ea893f2d
Коммит 6e54d85e3b
12 изменённых файлов: 24 добавлений и 190 удалений

Просмотреть файл

@ -442,9 +442,9 @@ const featureDescriptions = [
"Record File I/O markers from all threads, even unregistered threads.",
},
{
name: "No Marker Stacks",
value: "nomarkerstacks",
title: "Do not capture stacks when recording markers, to reduce overhead.",
name: "No File IO Stack Sampling",
value: "noiostacks",
title: "Do not sample stacks when recording File I/O markers.",
},
{
name: "Sequential Styling",

Просмотреть файл

@ -1126,12 +1126,6 @@ bool RacyFeatures::IsActiveWithFeature(uint32_t aFeature) {
return (af & Active) && (af & aFeature);
}
/* static */
bool RacyFeatures::IsActiveWithoutFeature(uint32_t aFeature) {
uint32_t af = sActiveAndFeatures; // copy it first
return (af & Active) && !(af & aFeature);
}
/* static */
bool RacyFeatures::IsActiveAndUnpaused() {
uint32_t af = sActiveAndFeatures; // copy it first
@ -3422,15 +3416,6 @@ bool profiler_feature_active(uint32_t aFeature) {
return RacyFeatures::IsActiveWithFeature(aFeature);
}
bool profiler_active_without_feature(uint32_t aFeature) {
// This function runs both on and off the main thread.
MOZ_RELEASE_ASSERT(CorePS::Exists());
// This function is hot enough that we use RacyFeatures, not ActivePS.
return RacyFeatures::IsActiveWithoutFeature(aFeature);
}
void profiler_add_sampled_counter(BaseProfilerCount* aCounter) {
DEBUG_LOG("profiler_add_sampled_counter(%s)", aCounter->mLabel);
PSAutoLock lock;
@ -3685,8 +3670,7 @@ UniquePtr<ProfileChunkedBuffer> profiler_capture_backtrace() {
PROFILER);
// Quick is-active check before allocating a buffer.
// If NoMarkerStacks is set, we don't want to capture a backtrace.
if (!profiler_active_without_feature(ProfilerFeature::NoMarkerStacks)) {
if (!profiler_is_active()) {
return nullptr;
}

Просмотреть файл

@ -60,10 +60,7 @@ ProfileBufferBlockIndex AddMarkerToBuffer(
AUTO_BASE_PROFILER_LABEL("baseprofiler::AddMarkerToBuffer", PROFILER);
return base_profiler_markers_detail::AddMarkerToBuffer<MarkerType>(
aBuffer, aName, aCategory, std::move(aOptions),
// Do not capture a stack if the NoMarkerStacks feature is set.
profiler_active_without_feature(ProfilerFeature::NoMarkerStacks)
? ::mozilla::baseprofiler::profiler_capture_backtrace_into
: nullptr,
::mozilla::baseprofiler::profiler_capture_backtrace_into,
aPayloadArguments...);
}

Просмотреть файл

@ -258,8 +258,8 @@ static ProfileBufferBlockIndex AddMarkerWithOptionalStackToBuffer(
// Pointer to a function that can capture a backtrace into the provided
// `ProfileChunkedBuffer`, and returns true when successful.
using OptionalBacktraceCaptureFunction = bool (*)(ProfileChunkedBuffer&,
StackCaptureOptions);
using BacktraceCaptureFunction = bool (*)(ProfileChunkedBuffer&,
StackCaptureOptions);
// Use a pre-allocated and cleared chunked buffer in the main thread's
// `AddMarkerToBuffer()`.
@ -276,8 +276,7 @@ template <typename MarkerType, typename... Ts>
ProfileBufferBlockIndex AddMarkerToBuffer(
ProfileChunkedBuffer& aBuffer, const ProfilerString8View& aName,
const MarkerCategory& aCategory, MarkerOptions&& aOptions,
OptionalBacktraceCaptureFunction aOptionalBacktraceCaptureFunction,
const Ts&... aTs) {
BacktraceCaptureFunction aBacktraceCaptureFunction, const Ts&... aTs) {
if (aOptions.ThreadId().IsUnspecified()) {
// If yet unspecified, set thread to this thread where the marker is added.
aOptions.Set(MarkerThreadId::CurrentThread());
@ -289,17 +288,14 @@ ProfileBufferBlockIndex AddMarkerToBuffer(
}
StackCaptureOptions captureOptions = aOptions.Stack().CaptureOptions();
if (captureOptions != StackCaptureOptions::NoStack &&
// Backtrace capture function will be nullptr if the profiler
// NoMarkerStacks feature is set.
aOptionalBacktraceCaptureFunction != nullptr) {
if (captureOptions != StackCaptureOptions::NoStack) {
// A capture was requested, let's attempt to do it here&now. This avoids a
// lot of allocations that would be necessary if capturing a backtrace
// separately.
// TODO reduce internal profiler stack levels, see bug 1659872.
auto CaptureStackAndAddMarker = [&](ProfileChunkedBuffer& aChunkedBuffer) {
aOptions.StackRef().UseRequestedBacktrace(
aOptionalBacktraceCaptureFunction(aChunkedBuffer, captureOptions)
aBacktraceCaptureFunction(aChunkedBuffer, captureOptions)
? &aChunkedBuffer
: nullptr);
// This call must be made from here, while chunkedBuffer is in scope.

Просмотреть файл

@ -181,8 +181,8 @@ class MOZ_RAII AutoProfilerStats {
MACRO(4, "fileioall", FileIOAll, \
"Add file I/O from all threads, implies fileio") \
\
MACRO(5, "nomarkerstacks", NoMarkerStacks, \
"Markers do not capture stacks, to reduce overhead") \
MACRO(5, "noiostacks", NoIOStacks, \
"File I/O markers do not capture stacks, to reduce overhead") \
\
MACRO(6, "screenshots", Screenshots, \
"Take a snapshot of the window on every composition") \
@ -277,20 +277,10 @@ class RacyFeatures {
MFBT_API static void SetSamplingUnpaused();
[[nodiscard]] MFBT_API static mozilla::Maybe<uint32_t> FeaturesIfActive() {
if (uint32_t af = sActiveAndFeatures; af & Active) {
// Active, remove the Active&Paused bits to get all features.
return Some(af & ~(Active | Paused | SamplingPaused));
}
return Nothing();
}
[[nodiscard]] MFBT_API static bool IsActive();
[[nodiscard]] MFBT_API static bool IsActiveWithFeature(uint32_t aFeature);
[[nodiscard]] MFBT_API static bool IsActiveWithoutFeature(uint32_t aFeature);
// True if profiler is active, and not fully paused.
// Note that periodic sampling *could* be paused!
[[nodiscard]] MFBT_API static bool IsActiveAndUnpaused();
@ -374,24 +364,12 @@ MFBT_API bool IsThreadBeingProfiled();
// not.
[[nodiscard]] MFBT_API uint32_t profiler_get_available_features();
// Returns the full feature set if the profiler is active.
// Note: the return value can become immediately out-of-date, much like the
// return value of profiler_is_active().
[[nodiscard]] inline mozilla::Maybe<uint32_t> profiler_features_if_active() {
return baseprofiler::detail::RacyFeatures::FeaturesIfActive();
}
// Check if a profiler feature (specified via the ProfilerFeature type) is
// active. Returns false if the profiler is inactive. Note: the return value
// can become immediately out-of-date, much like the return value of
// profiler_is_active().
[[nodiscard]] MFBT_API bool profiler_feature_active(uint32_t aFeature);
// Check if the profiler is active without a feature (specified via the
// ProfilerFeature type). Note: the return value can become immediately
// out-of-date, much like the return value of profiler_is_active().
[[nodiscard]] MFBT_API bool profiler_active_without_feature(uint32_t aFeature);
// Returns true if any of the profiler mutexes are currently locked *on the
// current thread*. This may be used by re-entrant code that may call profiler
// functions while the same of a different profiler mutex is locked, which could

Просмотреть файл

@ -28,7 +28,7 @@
"mainthreadio",
"fileio",
"fileioall",
"nomarkerstacks",
"noiostacks",
"screenshots",
"seqstyle",
"stackwalk",

Просмотреть файл

@ -346,9 +346,7 @@ void gecko_profiler_add_marker(
mozilla::StackCaptureOptions captureOptions =
markerOptions.Stack().CaptureOptions();
if (captureOptions != mozilla::StackCaptureOptions::NoStack &&
// Do not capture a stack if the NoMarkerStacks feature is set.
profiler_active_without_feature(ProfilerFeature::NoMarkerStacks)) {
if (captureOptions != mozilla::StackCaptureOptions::NoStack) {
// A capture was requested, let's attempt to do it here&now. This avoids a
// lot of allocations that would be necessary if capturing a backtrace
// separately.

Просмотреть файл

@ -6286,15 +6286,6 @@ bool profiler_feature_active(uint32_t aFeature) {
return RacyFeatures::IsActiveWithFeature(aFeature);
}
bool profiler_active_without_feature(uint32_t aFeature) {
// This function runs both on and off the main thread.
MOZ_RELEASE_ASSERT(CorePS::Exists());
// This function is hot enough that we use RacyFeatures, not ActivePS.
return RacyFeatures::IsActiveWithoutFeature(aFeature);
}
void profiler_write_active_configuration(JSONWriter& aWriter) {
MOZ_RELEASE_ASSERT(CorePS::Exists());
PSAutoLock lock;
@ -6795,9 +6786,8 @@ UniquePtr<ProfileChunkedBuffer> profiler_capture_backtrace() {
MOZ_RELEASE_ASSERT(CorePS::Exists());
AUTO_PROFILER_LABEL("profiler_capture_backtrace", PROFILER);
// Quick is-active and feature check before allocating a buffer.
// If NoMarkerStacks is set, we don't want to capture a backtrace.
if (!profiler_active_without_feature(ProfilerFeature::NoMarkerStacks)) {
// Quick is-active check before allocating a buffer.
if (!profiler_is_active()) {
return nullptr;
}

Просмотреть файл

@ -91,6 +91,7 @@ void ProfilerIOInterposeObserver::Observe(Observation& aObservation) {
}
AUTO_PROFILER_LABEL("ProfilerIOInterposeObserver", PROFILER);
const bool doCaptureStack = !(features & ProfilerFeature::NoIOStacks);
if (IsMainThread()) {
// This is the main thread.
// Capture a marker if any "IO" feature is on.
@ -108,7 +109,7 @@ void ProfilerIOInterposeObserver::Observe(Observation& aObservation) {
type, OTHER,
MarkerOptions(
MarkerTiming::Interval(aObservation.Start(), aObservation.End()),
MarkerStack::Capture()),
MarkerStack::MaybeCapture(doCaptureStack)),
FileIOMarker,
// aOperation
ProfilerString8View::WrapNullTerminatedString(
@ -134,7 +135,8 @@ void ProfilerIOInterposeObserver::Observe(Observation& aObservation) {
// Share a backtrace between the marker on this thread, and the marker on
// the main thread.
UniquePtr<ProfileChunkedBuffer> backtrace = profiler_capture_backtrace();
UniquePtr<ProfileChunkedBuffer> backtrace =
doCaptureStack ? profiler_capture_backtrace() : nullptr;
// Store the marker in the current thread.
PROFILER_MARKER(
@ -199,7 +201,7 @@ void ProfilerIOInterposeObserver::Observe(Observation& aObservation) {
type, OTHER,
MarkerOptions(
MarkerTiming::Interval(aObservation.Start(), aObservation.End()),
MarkerStack::Capture(),
doCaptureStack ? MarkerStack::Capture() : MarkerStack::NoStack(),
// Store this marker on the main thread.
MarkerThreadId::MainThread()),
FileIOMarker,

Просмотреть файл

@ -106,10 +106,7 @@ mozilla::ProfileBufferBlockIndex AddMarkerToBuffer(
mozilla::Unused << aMarkerType; // Only the empty object type is useful.
return mozilla::base_profiler_markers_detail::AddMarkerToBuffer<MarkerType>(
aBuffer, aName, aCategory, std::move(aOptions),
profiler_active_without_feature(ProfilerFeature::NoMarkerStacks)
? ::profiler_capture_backtrace_into
: nullptr,
aPayloadArguments...);
::profiler_capture_backtrace_into, aPayloadArguments...);
}
// Add a marker (without payload) to a given buffer.

Просмотреть файл

@ -61,8 +61,8 @@
MACRO(4, "fileioall", FileIOAll, \
"Add file I/O from all threads, implies fileio") \
\
MACRO(5, "nomarkerstacks", NoMarkerStacks, \
"Markers do not capture stacks, to reduce overhead") \
MACRO(5, "noiostacks", NoIOStacks, \
"File I/O markers do not capture stacks, to reduce overhead") \
\
MACRO(6, "screenshots", Screenshots, \
"Take a snapshot of the window on every composition") \
@ -266,11 +266,6 @@ class RacyFeatures {
return (af & Active) && (af & aFeature);
}
[[nodiscard]] static bool IsActiveWithoutFeature(uint32_t aFeature) {
uint32_t af = sActiveAndFeatures; // copy it first
return (af & Active) && !(af & aFeature);
}
// True if profiler is active, and not fully paused.
// Note that periodic sampling *could* be paused!
// This implementation must be kept in sync with
@ -372,11 +367,6 @@ profiler_features_if_active_and_unpaused() {
// profiler_is_active().
[[nodiscard]] bool profiler_feature_active(uint32_t aFeature);
// Check if the profiler is active without a feature (specified via the
// ProfilerFeature type). Note: the return value can become immediately
// out-of-date, much like the return value of profiler_is_active().
[[nodiscard]] bool profiler_active_without_feature(uint32_t aFeature);
// Returns true if any of the profiler mutexes are currently locked *on the
// current thread*. This may be used by re-entrant code that may call profiler
// functions while the same of a different profiler mutex is locked, which could

Просмотреть файл

@ -4978,102 +4978,4 @@ TEST(GeckoProfiler, FailureHandling)
ASSERT_EQ(profile.get(), nullptr);
}
TEST(GeckoProfiler, NoMarkerStacks)
{
uint32_t features = ProfilerFeature::NoMarkerStacks;
const char* filters[] = {"GeckoMain"};
ASSERT_TRUE(!profiler_get_profile());
// Make sure that profiler_capture_backtrace returns nullptr when the profiler
// is not active.
ASSERT_TRUE(!profiler_capture_backtrace());
{
// Start the profiler without the NoMarkerStacks feature and make sure we
// capture stacks.
profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
/* features */ 0, filters, MOZ_ARRAY_LENGTH(filters), 0);
ASSERT_TRUE(profiler_capture_backtrace());
profiler_stop();
}
// Start the profiler without the NoMarkerStacks feature and make sure we
// don't capture stacks.
profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL, features,
filters, MOZ_ARRAY_LENGTH(filters), 0);
// Make sure that the active features has the NoMarkerStacks feature.
mozilla::Maybe<uint32_t> activeFeatures = profiler_features_if_active();
ASSERT_TRUE(activeFeatures.isSome());
ASSERT_TRUE(ProfilerFeature::HasNoMarkerStacks(*activeFeatures));
// Make sure we don't capture stacks.
ASSERT_TRUE(!profiler_capture_backtrace());
// Add a marker with a stack to test.
EXPECT_TRUE(profiler_add_marker(
"Text with stack", geckoprofiler::category::OTHER, MarkerStack::Capture(),
geckoprofiler::markers::TextMarker{}, ""));
UniquePtr<char[]> profile = profiler_get_profile();
JSONOutputCheck(profile.get(), [&](const Json::Value& aRoot) {
// Check that the meta.configuration.features array contains
// "nomarkerstacks".
GET_JSON(meta, aRoot["meta"], Object);
{
GET_JSON(configuration, meta["configuration"], Object);
{
GET_JSON(features, configuration["features"], Array);
{
EXPECT_EQ(features.size(), 1u);
EXPECT_JSON_ARRAY_CONTAINS(features, String, "nomarkerstacks");
}
}
}
// Make sure that the marker we captured doesn't have a stack.
GET_JSON(threads, aRoot["threads"], Array);
{
ASSERT_EQ(threads.size(), 1u);
GET_JSON(thread0, threads[0], Object);
{
GET_JSON(markers, thread0["markers"], Object);
{
GET_JSON(data, markers["data"], Array);
{
const unsigned int NAME = 0u;
const unsigned int PAYLOAD = 5u;
bool foundMarker = false;
GET_JSON(stringTable, thread0["stringTable"], Array);
for (const Json::Value& marker : data) {
// Even though we only added one marker, some markers like
// NotifyObservers are being added as well. Let's iterate over
// them and make sure that we have the one we added explicitly and
// check its stack doesn't exist.
GET_JSON(name, stringTable[marker[NAME].asUInt()], String);
std::string nameString = name.asString();
if (nameString == "Text with stack") {
// Make sure that the marker doesn't have a stack.
foundMarker = true;
EXPECT_FALSE(marker[PAYLOAD].isNull());
EXPECT_TRUE(marker[PAYLOAD]["stack"].isNull());
}
}
EXPECT_TRUE(foundMarker);
}
}
}
}
});
profiler_stop();
ASSERT_TRUE(!profiler_get_profile());
}
#endif // MOZ_GECKO_PROFILER