Skip to content

Commit

Permalink
Backed out 3 changesets (bug 1734867) for causing xpcshell failures o…
Browse files Browse the repository at this point in the history
…n ProfileBufferChunkManager.h

Backed out changeset 63b2fd522aa8 (bug 1734867)
Backed out changeset 17219f7b60f5 (bug 1734867)
Backed out changeset 46bfc82686ac (bug 1734867)
  • Loading branch information
Cristian Tuns committed Dec 15, 2021
1 parent 07f6cf4 commit b046625
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 75 deletions.
13 changes: 3 additions & 10 deletions mozglue/baseprofiler/core/ProfileBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,10 @@ class ProfileBuffer final {
// - Adding JIT info.
// - Streaming stacks to JSON.
// Mutable because it's accessed from non-multithreaded const methods.
mutable Maybe<ProfileBufferChunkManagerSingle> mMaybeWorkerChunkManager;
ProfileBufferChunkManagerSingle& WorkerChunkManager() const {
if (mMaybeWorkerChunkManager.isNothing()) {
// Only actually allocate it on first use. (Some ProfileBuffers are
// temporary and don't actually need this.)
mMaybeWorkerChunkManager.emplace(
mutable ProfileBufferChunkManagerSingle mWorkerChunkManager{
ProfileBufferChunk::Create(
ProfileBufferChunk::SizeofChunkMetadata() +
ProfileBufferChunkManager::scExpectedMaximumStackSize);
}
return *mMaybeWorkerChunkManager;
}
ProfileBufferChunkManager::scExpectedMaximumStackSize)};

// Time from launch (us) when first sampling was recorded.
double mFirstSamplingTimeUs = 0.0;
Expand Down
4 changes: 2 additions & 2 deletions mozglue/baseprofiler/core/ProfileBufferEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1210,10 +1210,10 @@ bool ProfileBuffer::DuplicateLastSample(BaseProfilerThreadId aThreadId,
}

ProfileChunkedBuffer tempBuffer(
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, WorkerChunkManager());
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, mWorkerChunkManager);

auto retrieveWorkerChunk = MakeScopeExit(
[&]() { WorkerChunkManager().Reset(tempBuffer.GetAllChunks()); });
[&]() { mWorkerChunkManager.Reset(tempBuffer.GetAllChunks()); });

const bool ok = mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
MOZ_ASSERT(aReader,
Expand Down
23 changes: 0 additions & 23 deletions mozglue/baseprofiler/core/ProfilerMarkers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "mozilla/BaseProfilerMarkers.h"

#include "mozilla/Likely.h"
#include "mozilla/Maybe.h"

#include <limits>

Expand Down Expand Up @@ -75,28 +74,6 @@ Streaming::MarkerTypeFunctionsArray() {
return {sMarkerTypeFunctions1Based, sDeserializerCount};
}

struct ChunkedBufferWithSingleManager {
ProfileBufferChunkManagerSingle mChunkManager{
ProfileBufferChunkManager::scExpectedMaximumStackSize};
ProfileChunkedBuffer mChunkedBuffer{
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, mChunkManager};
};

static Maybe<ChunkedBufferWithSingleManager>
sMaybeChunkedBufferWithSingleManager;

ProfileChunkedBuffer& StaticClearedBufferForMainThreadAddMarker() {
// Once it's `emplaced` once, it will never happen again.
if (MOZ_UNLIKELY(sMaybeChunkedBufferWithSingleManager.isNothing())) {
sMaybeChunkedBufferWithSingleManager.emplace();
} else {
// Make sure the buffer is cleared -- This recycles the chunk from a
// previous call.
sMaybeChunkedBufferWithSingleManager->mChunkedBuffer.Clear();
}
return sMaybeChunkedBufferWithSingleManager->mChunkedBuffer;
}

} // namespace base_profiler_markers_detail

void MarkerSchema::Stream(JSONWriter& aWriter,
Expand Down
31 changes: 8 additions & 23 deletions mozglue/baseprofiler/public/BaseProfilerMarkersDetail.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,6 @@ static ProfileBufferBlockIndex AddMarkerWithOptionalStackToBuffer(
using BacktraceCaptureFunction = bool (*)(ProfileChunkedBuffer&,
StackCaptureOptions);

// Use a static (cleared) chunked buffer in the main thread's
// `AddMarkerToBuffer()`. This is a separate non-templated function, to ensure
// there is only one allocation, regardless of the type of marker.
MFBT_API ProfileChunkedBuffer& StaticClearedBufferForMainThreadAddMarker();

// Add a marker with the given name, options, and arguments to the given buffer.
// Because this may be called from either Base or Gecko Profiler functions, the
// appropriate backtrace-capturing function must also be provided.
Expand All @@ -281,29 +276,19 @@ ProfileBufferBlockIndex AddMarkerToBuffer(
// 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(
aBacktraceCaptureFunction(aChunkedBuffer, captureOptions)
? &aChunkedBuffer
: nullptr);
// This call must be made from here, while chunkedBuffer is in scope.
return AddMarkerWithOptionalStackToBuffer<MarkerType>(
aBuffer, aName, aCategory, std::move(aOptions), aTs...);
};

if (baseprofiler::profiler_is_main_thread()) {
// Use a static buffer for the main thread (because it's the most used
// thread, and most sensitive to overhead), so it's only allocated once.
return CaptureStackAndAddMarker(
StaticClearedBufferForMainThreadAddMarker());
}
// TODO use a local on-stack byte buffer to remove last allocation.
// TODO reduce internal profiler stack levels, see bug 1659872.
ProfileBufferChunkManagerSingle chunkManager(
ProfileBufferChunkManager::scExpectedMaximumStackSize);
ProfileChunkedBuffer chunkedBuffer(
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, chunkManager);
return CaptureStackAndAddMarker(chunkedBuffer);
aOptions.StackRef().UseRequestedBacktrace(
aBacktraceCaptureFunction(chunkedBuffer, captureOptions)
? &chunkedBuffer
: nullptr);
// This call must be made from here, while chunkedBuffer is in scope.
return AddMarkerWithOptionalStackToBuffer<MarkerType>(
aBuffer, aName, aCategory, std::move(aOptions), aTs...);
}

return AddMarkerWithOptionalStackToBuffer<MarkerType>(
Expand Down
14 changes: 3 additions & 11 deletions tools/profiler/core/ProfileBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,18 +178,10 @@ class ProfileBuffer final {
// - Adding JIT info.
// - Streaming stacks to JSON.
// Mutable because it's accessed from non-multithreaded const methods.
mutable mozilla::Maybe<mozilla::ProfileBufferChunkManagerSingle>
mMaybeWorkerChunkManager;
mozilla::ProfileBufferChunkManagerSingle& WorkerChunkManager() const {
if (mMaybeWorkerChunkManager.isNothing()) {
// Only actually allocate it on first use. (Some ProfileBuffers are
// temporary and don't actually need this.)
mMaybeWorkerChunkManager.emplace(
mutable mozilla::ProfileBufferChunkManagerSingle mWorkerChunkManager{
mozilla::ProfileBufferChunk::Create(
mozilla::ProfileBufferChunk::SizeofChunkMetadata() +
mozilla::ProfileBufferChunkManager::scExpectedMaximumStackSize);
}
return *mMaybeWorkerChunkManager;
}
mozilla::ProfileBufferChunkManager::scExpectedMaximumStackSize)};

// GetStreamingParametersForThreadCallback:
// (ProfilerThreadId) -> Maybe<StreamingParametersForThread>
Expand Down
12 changes: 6 additions & 6 deletions tools/profiler/core/ProfileBufferEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ ProfilerThreadId ProfileBuffer::DoStreamSamplesAndMarkersToJSON(
if (kind == ProfileBufferEntry::Kind::CompactStack) {
ProfileChunkedBuffer tempBuffer(
ProfileChunkedBuffer::ThreadSafety::WithoutMutex,
WorkerChunkManager());
mWorkerChunkManager);
er.ReadIntoObject(tempBuffer);
tempBuffer.Read([&](ProfileChunkedBuffer::Reader* aReader) {
MOZ_ASSERT(aReader,
Expand All @@ -1195,7 +1195,7 @@ ProfilerThreadId ProfileBuffer::DoStreamSamplesAndMarkersToJSON(
it.CurrentBlockIndex().ConvertToProfileBufferIndex(),
unresponsiveDuration, runningTimes);
});
WorkerChunkManager().Reset(tempBuffer.GetAllChunks());
mWorkerChunkManager.Reset(tempBuffer.GetAllChunks());
break;
}

Expand Down Expand Up @@ -1394,7 +1394,7 @@ void ProfileBuffer::AddJITInfoForRange(uint64_t aRangeStart,
if (kind == ProfileBufferEntry::Kind::CompactStack) {
ProfileChunkedBuffer tempBuffer(
ProfileChunkedBuffer::ThreadSafety::WithoutMutex,
WorkerChunkManager());
mWorkerChunkManager);
er.ReadIntoObject(tempBuffer);
tempBuffer.Read([&](ProfileChunkedBuffer::Reader* aReader) {
MOZ_ASSERT(
Expand All @@ -1408,7 +1408,7 @@ void ProfileBuffer::AddJITInfoForRange(uint64_t aRangeStart,
stackEntryGetter.Next();
}
});
WorkerChunkManager().Reset(tempBuffer.GetAllChunks());
mWorkerChunkManager.Reset(tempBuffer.GetAllChunks());
break;
}

Expand Down Expand Up @@ -1939,10 +1939,10 @@ bool ProfileBuffer::DuplicateLastSample(ProfilerThreadId aThreadId,
AUTO_PROFILER_STATS(DuplicateLastSample_copy);

ProfileChunkedBuffer tempBuffer(
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, WorkerChunkManager());
ProfileChunkedBuffer::ThreadSafety::WithoutMutex, mWorkerChunkManager);

auto retrieveWorkerChunk = MakeScopeExit(
[&]() { WorkerChunkManager().Reset(tempBuffer.GetAllChunks()); });
[&]() { mWorkerChunkManager.Reset(tempBuffer.GetAllChunks()); });

const bool ok = mEntries.Read([&](ProfileChunkedBuffer::Reader* aReader) {
MOZ_ASSERT(aReader,
Expand Down

0 comments on commit b046625

Please sign in to comment.