[scudo] Remove unused field in BatchGroup (#109322)

Also fix the logic while determining the size of BatchClass and update
some legacy comments.
This commit is contained in:
ChiaHungDuan
2024-09-19 13:58:47 -07:00
committed by GitHub
parent 21026073e3
commit 0347c11226
3 changed files with 14 additions and 43 deletions

View File

@@ -75,16 +75,13 @@ template <class SizeClassAllocator> struct BatchGroup {
BatchGroup *Next;
// The compact base address of each group
uptr CompactPtrGroupBase;
// Cache value of SizeClassAllocatorLocalCache::getMaxCached()
u16 MaxCachedPerBatch;
// Number of blocks pushed into this group. This is an increment-only
// counter.
uptr PushedBlocks;
// This is used to track how many bytes are not in-use since last time we
// tried to release pages.
uptr BytesInBGAtLastCheckpoint;
// Blocks are managed by TransferBatch in a list.
SinglyLinkedList<TransferBatch<SizeClassAllocator>> Batches;
// Cache value of SizeClassAllocatorLocalCache::getMaxCached()
u16 MaxCachedPerBatch;
};
} // namespace scudo

View File

@@ -56,12 +56,9 @@ public:
typedef TransferBatch<ThisT> TransferBatchT;
typedef BatchGroup<ThisT> BatchGroupT;
static_assert(sizeof(BatchGroupT) <= sizeof(TransferBatchT),
"BatchGroupT uses the same class size as TransferBatchT");
static uptr getSizeByClassId(uptr ClassId) {
return (ClassId == SizeClassMap::BatchClassId)
? sizeof(TransferBatchT)
? Max(sizeof(BatchGroupT), sizeof(TransferBatchT))
: SizeClassMap::getSizeByClassId(ClassId);
}
@@ -531,9 +528,6 @@ private:
// BatchClass hasn't enabled memory group. Use `0` to indicate there's no
// memory group here.
BG->CompactPtrGroupBase = 0;
// `BG` is also the block of BatchClassId. Note that this is different
// from `CreateGroup` in `pushBlocksImpl`
BG->PushedBlocks = 1;
BG->BytesInBGAtLastCheckpoint = 0;
BG->MaxCachedPerBatch =
CacheT::getMaxCached(getSizeByClassId(SizeClassMap::BatchClassId));
@@ -558,9 +552,6 @@ private:
TB->add(
compactPtr(SizeClassMap::BatchClassId, reinterpret_cast<uptr>(BG)));
--Size;
DCHECK_EQ(BG->PushedBlocks, 1U);
// `TB` is also the block of BatchClassId.
BG->PushedBlocks += 1;
BG->Batches.push_front(TB);
}
@@ -587,8 +578,6 @@ private:
CurBatch->appendFromArray(&Array[I], AppendSize);
I += AppendSize;
}
BG->PushedBlocks += Size;
}
// Push the blocks to their batch group. The layout will be like,
//
@@ -624,7 +613,6 @@ private:
BG->CompactPtrGroupBase = CompactPtrGroupBase;
BG->Batches.push_front(TB);
BG->PushedBlocks = 0;
BG->BytesInBGAtLastCheckpoint = 0;
BG->MaxCachedPerBatch = TransferBatchT::MaxNumCached;
@@ -652,8 +640,6 @@ private:
CurBatch->appendFromArray(&Array[I], AppendSize);
I += AppendSize;
}
BG->PushedBlocks += Size;
};
Sci->FreeListInfo.PushedBlocks += Size;

View File

@@ -61,12 +61,12 @@ public:
typedef TransferBatch<ThisT> TransferBatchT;
typedef BatchGroup<ThisT> BatchGroupT;
static_assert(sizeof(BatchGroupT) <= sizeof(TransferBatchT),
"BatchGroupT uses the same class size as TransferBatchT");
// BachClass is used to store internal metadata so it needs to be at least as
// large as the largest data structure.
static uptr getSizeByClassId(uptr ClassId) {
return (ClassId == SizeClassMap::BatchClassId)
? roundUp(sizeof(TransferBatchT), 1U << CompactPtrScale)
? roundUp(Max(sizeof(TransferBatchT), sizeof(BatchGroupT)),
1U << CompactPtrScale)
: SizeClassMap::getSizeByClassId(ClassId);
}
@@ -236,7 +236,7 @@ public:
} else {
while (true) {
// When two threads compete for `Region->MMLock`, we only want one of
// them to call populateFreeListAndPopBatch(). To avoid both of them
// them to call populateFreeListAndPopBlocks(). To avoid both of them
// doing that, always check the freelist before mapping new pages.
ScopedLock ML(Region->MMLock);
{
@@ -690,9 +690,6 @@ private:
// BatchClass hasn't enabled memory group. Use `0` to indicate there's no
// memory group here.
BG->CompactPtrGroupBase = 0;
// `BG` is also the block of BatchClassId. Note that this is different
// from `CreateGroup` in `pushBlocksImpl`
BG->PushedBlocks = 1;
BG->BytesInBGAtLastCheckpoint = 0;
BG->MaxCachedPerBatch =
CacheT::getMaxCached(getSizeByClassId(SizeClassMap::BatchClassId));
@@ -717,9 +714,6 @@ private:
TB->add(
compactPtr(SizeClassMap::BatchClassId, reinterpret_cast<uptr>(BG)));
--Size;
DCHECK_EQ(BG->PushedBlocks, 1U);
// `TB` is also the block of BatchClassId.
BG->PushedBlocks += 1;
BG->Batches.push_front(TB);
}
@@ -746,8 +740,6 @@ private:
CurBatch->appendFromArray(&Array[I], AppendSize);
I += AppendSize;
}
BG->PushedBlocks += Size;
}
// Push the blocks to their batch group. The layout will be like,
@@ -782,7 +774,6 @@ private:
BG->CompactPtrGroupBase = CompactPtrGroupBase;
BG->Batches.push_front(TB);
BG->PushedBlocks = 0;
BG->BytesInBGAtLastCheckpoint = 0;
BG->MaxCachedPerBatch = TransferBatchT::MaxNumCached;
@@ -810,8 +801,6 @@ private:
CurBatch->appendFromArray(&Array[I], AppendSize);
I += AppendSize;
}
BG->PushedBlocks += Size;
};
Region->FreeListInfo.PushedBlocks += Size;
@@ -884,7 +873,7 @@ private:
while (true) {
// We only expect one thread doing the freelist refillment and other
// threads will be waiting for either the completion of the
// `populateFreeListAndPopBatch()` or `pushBlocks()` called by other
// `populateFreeListAndPopBlocks()` or `pushBlocks()` called by other
// threads.
bool PopulateFreeList = false;
{
@@ -922,7 +911,7 @@ private:
// At here, there are two preconditions to be met before waiting,
// 1. The freelist is empty.
// 2. Region->isPopulatingFreeList == true, i.e, someone is still doing
// `populateFreeListAndPopBatch()`.
// `populateFreeListAndPopBlocks()`.
//
// Note that it has the chance that freelist is empty but
// Region->isPopulatingFreeList == false because all the new populated
@@ -938,8 +927,8 @@ private:
// Now the freelist is empty and someone's doing the refillment. We will
// wait until anyone refills the freelist or someone finishes doing
// `populateFreeListAndPopBatch()`. The refillment can be done by
// `populateFreeListAndPopBatch()`, `pushBlocks()`,
// `populateFreeListAndPopBlocks()`. The refillment can be done by
// `populateFreeListAndPopBlocks()`, `pushBlocks()`,
// `pushBatchClassBlocks()` and `mergeGroupsToReleaseBack()`.
Region->FLLockCV.wait(Region->FLLock);
@@ -1119,8 +1108,8 @@ private:
// Note that `PushedBlocks` and `PoppedBlocks` are supposed to only record
// the requests from `PushBlocks` and `PopBatch` which are external
// interfaces. `populateFreeListAndPopBatch` is the internal interface so we
// should set the values back to avoid incorrectly setting the stats.
// interfaces. `populateFreeListAndPopBlocks` is the internal interface so
// we should set the values back to avoid incorrectly setting the stats.
Region->FreeListInfo.PushedBlocks -= NumberOfBlocks;
const uptr AllocatedUser = Size * NumberOfBlocks;
@@ -1689,7 +1678,6 @@ private:
GroupsToRelease.pop_front();
if (BG->CompactPtrGroupBase == Cur->CompactPtrGroupBase) {
BG->PushedBlocks += Cur->PushedBlocks;
// We have updated `BatchGroup::BytesInBGAtLastCheckpoint` while
// collecting the `GroupsToRelease`.
BG->BytesInBGAtLastCheckpoint = Cur->BytesInBGAtLastCheckpoint;