Bug 1883177 - Move segment cleanup responsibility to callers, r=xpcom-reviewers,jstutte

Before this change, the nsSegmentedBuffer would automatically dispatch
the `free` call for each individual segment to another thread, aiming to
avoid jank when freeing a large number of buffers on the main thread.
This changes that behaviour such that the responsibility to deal with
the underlying buffer is now on the caller, which has more context about
usage patterns and whether or not to batch the free calls.

Differential Revision: https://phabricator.services.mozilla.com/D235246
This commit is contained in:
Nika Layzell
2025-02-07 21:31:43 +00:00
parent 4197a454e1
commit d0de9309d6
5 changed files with 91 additions and 132 deletions

View File

@@ -128,9 +128,10 @@ class CallbackHolder {
//-----------------------------------------------------------------------------
// this class is used to delay notifications until the end of a particular
// scope. it helps avoid the complexity of issuing callbacks while inside
// a critical section.
// This class is used to delay notifications until the end of a particular
// scope. It helps to avoid the complexity of issuing callbacks while inside
// a critical section. It also holds a list of segments that have become
// obsolete during that particular scope to be bulk-freed later.
class nsPipeEvents {
public:
nsPipeEvents() = default;
@@ -140,8 +141,13 @@ class nsPipeEvents {
mCallbacks.AppendElement(std::move(aCallback));
}
inline void FreeSegment(mozilla::UniqueFreePtr<char> aSegment) {
mSegmentsToFree.AppendElement(std::move(aSegment));
}
private:
nsTArray<CallbackHolder> mCallbacks;
AutoTArray<CallbackHolder, 4> mCallbacks;
AutoTArray<mozilla::UniqueFreePtr<char>, 4> mSegmentsToFree;
};
//-----------------------------------------------------------------------------
@@ -353,6 +359,7 @@ class nsPipe final {
char*& aCursor, char*& aLimit)
MOZ_REQUIRES(mReentrantMonitor);
SegmentChangeResult AdvanceReadSegment(nsPipeReadState& aReadState,
nsPipeEvents& aEvents,
const ReentrantMonitorAutoEnter& ev)
MOZ_REQUIRES(mReentrantMonitor);
bool ReadSegmentBeingWritten(nsPipeReadState& aReadState)
@@ -666,7 +673,8 @@ void nsPipe::AdvanceReadCursor(nsPipeReadState& aReadState,
// Advance the segment position. If we have read any segments from the
// advance buffer then we can potentially notify blocked writers.
mOutput.Monitor().AssertCurrentThreadIn();
if (AdvanceReadSegment(aReadState, mon) == SegmentAdvanceBufferRead &&
if (AdvanceReadSegment(aReadState, events, mon) ==
SegmentAdvanceBufferRead &&
mOutput.OnOutputWritable(events) == NotifyMonitor) {
mon.NotifyAll();
}
@@ -677,7 +685,8 @@ void nsPipe::AdvanceReadCursor(nsPipeReadState& aReadState,
}
SegmentChangeResult nsPipe::AdvanceReadSegment(
nsPipeReadState& aReadState, const ReentrantMonitorAutoEnter& ev) {
nsPipeReadState& aReadState, nsPipeEvents& aEvents,
const ReentrantMonitorAutoEnter& ev) {
// Calculate how many segments are buffered for this stream to start.
uint32_t startBufferSegments = GetBufferSegmentCount(aReadState, ev);
@@ -706,7 +715,7 @@ SegmentChangeResult nsPipe::AdvanceReadSegment(
}
// done with this segment
mBuffer.DeleteFirstSegment();
aEvents.FreeSegment(mBuffer.PopFirstSegment());
LOG(("III deleting first segment\n"));
}
@@ -770,7 +779,7 @@ void nsPipe::DrainInputStream(nsPipeReadState& aReadState,
// Don't bother checking if this results in an advance buffer segment
// read. Since we are draining the entire stream we will read an
// advance buffer segment no matter what.
AdvanceReadSegment(aReadState, mon);
AdvanceReadSegment(aReadState, aEvents, mon);
}
// Force the stream into an empty state. Make sure mAvailable, mCursor, and
@@ -1155,6 +1164,17 @@ nsPipeEvents::~nsPipeEvents() {
callback.Notify();
}
mCallbacks.Clear();
// free segments which are no longer required. use a background task if we're
// freeing a large number of segments to avoid blocking the current thread.
if (mSegmentsToFree.Length() > 128) {
NS_DispatchBackgroundTask(NS_NewRunnableFunction(
"nsPipe FreeSegments",
[segments = std::move(mSegmentsToFree)]() mutable {
segments.Clear();
}));
}
mSegmentsToFree.Clear();
}
//-----------------------------------------------------------------------------

View File

@@ -10,6 +10,8 @@
#include "nsThreadUtils.h"
#include "mozilla/ScopeExit.h"
static constexpr uint32_t kSegmentedBufferFreeOMTThreshold = 128;
nsresult nsSegmentedBuffer::Init(uint32_t aSegmentSize) {
if (mSegmentArrayCount != 0) {
return NS_ERROR_FAILURE; // initialized more than once
@@ -19,7 +21,8 @@ nsresult nsSegmentedBuffer::Init(uint32_t aSegmentSize) {
return NS_OK;
}
char* nsSegmentedBuffer::AppendNewSegment() {
char* nsSegmentedBuffer::AppendNewSegment(
mozilla::UniqueFreePtr<char> aSegment) {
if (!mSegmentArray) {
uint32_t bytes = mSegmentArrayCount * sizeof(char*);
mSegmentArray = (char**)moz_xmalloc(bytes);
@@ -51,7 +54,7 @@ char* nsSegmentedBuffer::AppendNewSegment() {
mSegmentArrayCount = newArraySize.value();
}
char* seg = (char*)malloc(mSegmentSize);
char* seg = aSegment ? aSegment.release() : (char*)malloc(mSegmentSize);
if (!seg) {
return nullptr;
}
@@ -60,28 +63,27 @@ char* nsSegmentedBuffer::AppendNewSegment() {
return seg;
}
bool nsSegmentedBuffer::DeleteFirstSegment() {
mozilla::UniqueFreePtr<char> nsSegmentedBuffer::PopFirstSegment() {
NS_ASSERTION(mSegmentArray[mFirstSegmentIndex] != nullptr,
"deleting bad segment");
FreeOMT(mSegmentArray[mFirstSegmentIndex]);
mozilla::UniqueFreePtr<char> segment(mSegmentArray[mFirstSegmentIndex]);
mSegmentArray[mFirstSegmentIndex] = nullptr;
int32_t last = ModSegArraySize(mLastSegmentIndex - 1);
if (mFirstSegmentIndex == last) {
mLastSegmentIndex = last;
return true;
} else {
mFirstSegmentIndex = ModSegArraySize(mFirstSegmentIndex + 1);
return false;
}
return segment;
}
bool nsSegmentedBuffer::DeleteLastSegment() {
mozilla::UniqueFreePtr<char> nsSegmentedBuffer::PopLastSegment() {
int32_t last = ModSegArraySize(mLastSegmentIndex - 1);
NS_ASSERTION(mSegmentArray[last] != nullptr, "deleting bad segment");
FreeOMT(mSegmentArray[last]);
mozilla::UniqueFreePtr<char> segment(mSegmentArray[last]);
mSegmentArray[last] = nullptr;
mLastSegmentIndex = last;
return (bool)(mLastSegmentIndex == mFirstSegmentIndex);
return segment;
}
bool nsSegmentedBuffer::ReallocLastSegment(size_t aNewSize) {
@@ -95,75 +97,31 @@ bool nsSegmentedBuffer::ReallocLastSegment(size_t aNewSize) {
return false;
}
void nsSegmentedBuffer::Empty() {
auto clearMembers = mozilla::MakeScopeExit([&] {
mSegmentArray = nullptr;
mSegmentArrayCount = NS_SEGMENTARRAY_INITIAL_COUNT;
void nsSegmentedBuffer::Clear() {
// Clear out the buffer's members back to their initial state.
uint32_t arrayCount =
std::exchange(mSegmentArrayCount, NS_SEGMENTARRAY_INITIAL_COUNT);
char** segmentArray = std::exchange(mSegmentArray, nullptr);
mFirstSegmentIndex = mLastSegmentIndex = 0;
});
// If mSegmentArray is null, there's no need to actually free anything
if (!mSegmentArray) {
return;
}
// Dispatch a task that frees up the array. This may run immediately or on
// a background thread.
FreeOMT([segmentArray = mSegmentArray, arrayCount = mSegmentArrayCount]() {
for (uint32_t i = 0; i < arrayCount; i++) {
auto freeSegmentArray = [arrayCount, segmentArray]() {
for (uint32_t i = 0; i < arrayCount; ++i) {
if (segmentArray[i]) {
free(segmentArray[i]);
}
}
free(segmentArray);
});
}
};
void nsSegmentedBuffer::FreeOMT(void* aPtr) {
FreeOMT([aPtr]() { free(aPtr); });
if (segmentArray) {
// If we have a small number of entries, free them synchronously. In some
// rare cases, `nsSegmentedBuffer` may be gigantic, in which case it should
// be freed async in a background task to avoid janking this thread.
if (arrayCount < kSegmentedBufferFreeOMTThreshold ||
NS_FAILED(NS_DispatchBackgroundTask(NS_NewRunnableFunction(
"nsSegmentedBuffer::Clear", freeSegmentArray)))) {
freeSegmentArray();
}
void nsSegmentedBuffer::FreeOMT(std::function<void()>&& aTask) {
if (!NS_IsMainThread()) {
aTask();
return;
}
if (mFreeOMT) {
// There is a runnable pending which will handle this object
if (mFreeOMT->AddTask(std::move(aTask)) > 1) {
return;
}
} else {
mFreeOMT = mozilla::MakeRefPtr<FreeOMTPointers>();
mFreeOMT->AddTask(std::move(aTask));
}
if (!mIOThread) {
mIOThread = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
}
// During the shutdown we are not able to obtain the IOThread and/or the
// dispatching of runnable fails.
if (!mIOThread || NS_FAILED(mIOThread->Dispatch(NS_NewRunnableFunction(
"nsSegmentedBuffer::FreeOMT",
[obj = mFreeOMT]() { obj->FreeAll(); })))) {
mFreeOMT->FreeAll();
}
}
void nsSegmentedBuffer::FreeOMTPointers::FreeAll() {
// Take all the tasks from the object. If AddTask is called after we release
// the lock, then another runnable will be dispatched for that task. This is
// necessary to avoid blocking the main thread while memory is being freed.
nsTArray<std::function<void()>> tasks = [this]() {
auto t = mTasks.Lock();
return std::move(*t);
}();
// Finally run all the tasks to free memory.
for (auto& task : tasks) {
task();
}
}

View File

@@ -15,6 +15,7 @@
#include "nsError.h"
#include "nsTArray.h"
#include "mozilla/DataMutex.h"
#include "mozilla/UniquePtrExtensions.h"
class nsIEventTarget;
@@ -27,23 +28,25 @@ class nsSegmentedBuffer {
mFirstSegmentIndex(0),
mLastSegmentIndex(0) {}
~nsSegmentedBuffer() { Empty(); }
~nsSegmentedBuffer() { Clear(); }
nsresult Init(uint32_t aSegmentSize);
char* AppendNewSegment(); // pushes at end
// pushes at end
// aSegment must either be null or at least mSegmentSize bytes large.
char* AppendNewSegment(mozilla::UniqueFreePtr<char> aSegment = nullptr);
// returns true if no more segments remain:
bool DeleteFirstSegment(); // pops from beginning
// pops from beginning, and returns the segment
mozilla::UniqueFreePtr<char> PopFirstSegment();
// returns true if no more segments remain:
bool DeleteLastSegment(); // pops from beginning
// pops from end, and returns the segment
mozilla::UniqueFreePtr<char> PopLastSegment();
// Call Realloc() on last segment. This is used to reduce memory
// consumption when data is not an exact multiple of segment size.
bool ReallocLastSegment(size_t aNewSize);
void Empty(); // frees all segments
void Clear(); // frees all segments
inline uint32_t GetSegmentCount() {
if (mFirstSegmentIndex <= mLastSegmentIndex) {
@@ -79,38 +82,6 @@ class nsSegmentedBuffer {
uint32_t mSegmentArrayCount;
int32_t mFirstSegmentIndex;
int32_t mLastSegmentIndex;
private:
class FreeOMTPointers {
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FreeOMTPointers)
public:
FreeOMTPointers() : mTasks("nsSegmentedBuffer::FreeOMTPointers") {}
void FreeAll();
// Adds a task to the array. Returns the size of the array.
size_t AddTask(std::function<void()>&& aTask) {
auto tasks = mTasks.Lock();
tasks->AppendElement(std::move(aTask));
return tasks->Length();
}
private:
~FreeOMTPointers() = default;
mozilla::DataMutex<nsTArray<std::function<void()>>> mTasks;
};
void FreeOMT(void* aPtr);
void FreeOMT(std::function<void()>&& aTask);
nsCOMPtr<nsIEventTarget> mIOThread;
// This object is created the first time we need to dispatch to another thread
// to free segments. It is only freed when the nsSegmentedBufer is destroyed
// or when the runnable is finally handled and its refcount goes to 0.
RefPtr<FreeOMTPointers> mFreeOMT;
};
// NS_SEGMENTARRAY_INITIAL_SIZE: This number needs to start out as a

View File

@@ -283,11 +283,21 @@ nsresult nsStorageStream::SetLengthLocked(uint32_t aLength) {
newLastSegmentNum--;
}
AutoTArray<mozilla::UniqueFreePtr<char>, 16> toFree;
while (newLastSegmentNum < mLastSegmentNum) {
mSegmentedBuffer->DeleteLastSegment();
toFree.AppendElement(mSegmentedBuffer->PopLastSegment());
mLastSegmentNum--;
}
// If we removed a substantial number of segments, dispatch the `free(...)`
// calls to a background thread to avoid potential calling thread jank.
if (toFree.Length() > 128) {
NS_DispatchBackgroundTask(NS_NewRunnableFunction(
"nsStorageStream::SetLengthLocked",
[toFree = std::move(toFree)]() mutable { toFree.Clear(); }));
}
mLogicalLength = aLength;
return NS_OK;
}

View File

@@ -8,34 +8,34 @@
using namespace mozilla;
TEST(SegmentedBuffer, AppendAndDelete)
TEST(SegmentedBuffer, AppendAndPop)
{
auto buf = MakeUnique<nsSegmentedBuffer>();
buf->Init(4);
char* seg;
bool empty;
mozilla::UniqueFreePtr<char> poppedSeg;
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(!empty) << "DeleteFirstSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(!empty) << "DeleteFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
seg = buf->AppendNewSegment();
EXPECT_TRUE(seg) << "AppendNewSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(!empty) << "DeleteFirstSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(!empty) << "DeleteFirstSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(!empty) << "DeleteFirstSegment failed";
empty = buf->DeleteFirstSegment();
EXPECT_TRUE(empty) << "DeleteFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
poppedSeg = buf->PopFirstSegment();
EXPECT_TRUE(poppedSeg) << "PopFirstSegment failed";
}