Skip to content

Commit 1ff3a5d

Browse files
committed
[scudo] Allow pushing single block to the freelist of BatchClass
This CL removes the restriction that pushing blocks into BatchClassId can only be done when freelist is not empty. Without this constraint, BatchClassId is also available for gathering blocks into groups. Reviewed By: cferris Differential Revision: https://reviews.llvm.org/D153492
1 parent b78b36e commit 1ff3a5d

File tree

2 files changed

+302
-211
lines changed

2 files changed

+302
-211
lines changed

compiler-rt/lib/scudo/standalone/primary32.h

Lines changed: 146 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,29 @@ template <typename Config> class SizeClassAllocator32 {
135135

136136
const uptr BlockSize = getSizeByClassId(I);
137137
DCHECK_EQ(TotalBlocks, Sci->AllocatedUser / BlockSize);
138+
DCHECK_EQ(Sci->FreeListInfo.PushedBlocks, Sci->FreeListInfo.PoppedBlocks);
138139
}
139140

140141
SizeClassInfo *Sci = getSizeClassInfo(SizeClassMap::BatchClassId);
141142
ScopedLock L1(Sci->Mutex);
142143
uptr TotalBlocks = 0;
143-
for (BatchGroup &BG : Sci->FreeListInfo.BlockList)
144-
for (const auto &It : BG.Batches)
145-
TotalBlocks += It.getCount();
144+
for (BatchGroup &BG : Sci->FreeListInfo.BlockList) {
145+
if (LIKELY(!BG.Batches.empty())) {
146+
for (const auto &It : BG.Batches)
147+
TotalBlocks += It.getCount();
148+
} else {
149+
// `BatchGroup` with empty freelist doesn't have `TransferBatch` record
150+
// itself.
151+
++TotalBlocks;
152+
}
153+
}
146154

147155
const uptr BlockSize = getSizeByClassId(SizeClassMap::BatchClassId);
148156
DCHECK_EQ(TotalBlocks + BatchClassUsedInFreeLists,
149157
Sci->AllocatedUser / BlockSize);
158+
const uptr BlocksInUse =
159+
Sci->FreeListInfo.PoppedBlocks - Sci->FreeListInfo.PushedBlocks;
160+
DCHECK_EQ(BlocksInUse, BatchClassUsedInFreeLists);
150161
}
151162

152163
CompactPtrT compactPtr(UNUSED uptr ClassId, uptr Ptr) const {
@@ -199,15 +210,7 @@ template <typename Config> class SizeClassAllocator32 {
199210
SizeClassInfo *Sci = getSizeClassInfo(ClassId);
200211
if (ClassId == SizeClassMap::BatchClassId) {
201212
ScopedLock L(Sci->Mutex);
202-
// Constructing a batch group in the free list will use two blocks in
203-
// BatchClassId. If we are pushing BatchClassId blocks, we will use the
204-
// blocks in the array directly (can't delegate local cache which will
205-
// cause a recursive allocation). However, The number of free blocks may
206-
// be less than two. Therefore, populate the free list before inserting
207-
// the blocks.
208-
if (Size == 1 && !populateFreeList(C, ClassId, Sci))
209-
return;
210-
pushBlocksImpl(C, ClassId, Sci, Array, Size);
213+
pushBatchClassBlocks(Sci, Array, Size);
211214
return;
212215
}
213216

@@ -450,6 +453,116 @@ template <typename Config> class SizeClassAllocator32 {
450453
return &SizeClassInfoArray[ClassId];
451454
}
452455

456+
void pushBatchClassBlocks(SizeClassInfo *Sci, CompactPtrT *Array, u32 Size)
457+
REQUIRES(Sci->Mutex) {
458+
DCHECK_EQ(Sci, getSizeClassInfo(SizeClassMap::BatchClassId));
459+
460+
// Free blocks are recorded by TransferBatch in freelist for all
461+
// size-classes. In addition, TransferBatch is allocated from BatchClassId.
462+
// In order not to use additional block to record the free blocks in
463+
// BatchClassId, they are self-contained. I.e., A TransferBatch records the
464+
// block address of itself. See the figure below:
465+
//
466+
// TransferBatch at 0xABCD
467+
// +----------------------------+
468+
// | Free blocks' addr |
469+
// | +------+------+------+ |
470+
// | |0xABCD|... |... | |
471+
// | +------+------+------+ |
472+
// +----------------------------+
473+
//
474+
// When we allocate all the free blocks in the TransferBatch, the block used
475+
// by TransferBatch is also free for use. We don't need to recycle the
476+
// TransferBatch. Note that the correctness is maintained by the invariant,
477+
//
478+
// The unit of each popBatch() request is entire TransferBatch. Return
479+
// part of the blocks in a TransferBatch is invalid.
480+
//
481+
// This ensures that TransferBatch won't leak the address itself while it's
482+
// still holding other valid data.
483+
//
484+
// Besides, BatchGroup is also allocated from BatchClassId and has its
485+
// address recorded in the TransferBatch too. To maintain the correctness,
486+
//
487+
// The address of BatchGroup is always recorded in the last TransferBatch
488+
// in the freelist (also imply that the freelist should only be
489+
// updated with push_front). Once the last TransferBatch is popped,
490+
// the block used by BatchGroup is also free for use.
491+
//
492+
// With this approach, the blocks used by BatchGroup and TransferBatch are
493+
// reusable and don't need additional space for them.
494+
495+
Sci->FreeListInfo.PushedBlocks += Size;
496+
BatchGroup *BG = Sci->FreeListInfo.BlockList.front();
497+
498+
if (BG == nullptr) {
499+
// Construct `BatchGroup` on the last element.
500+
BG = reinterpret_cast<BatchGroup *>(
501+
decompactPtr(SizeClassMap::BatchClassId, Array[Size - 1]));
502+
--Size;
503+
BG->Batches.clear();
504+
// BatchClass hasn't enabled memory group. Use `0` to indicate there's no
505+
// memory group here.
506+
BG->CompactPtrGroupBase = 0;
507+
// `BG` is also the block of BatchClassId. Note that this is different
508+
// from `CreateGroup` in `pushBlocksImpl`
509+
BG->PushedBlocks = 1;
510+
BG->BytesInBGAtLastCheckpoint = 0;
511+
BG->MaxCachedPerBatch = TransferBatch::getMaxCached(
512+
getSizeByClassId(SizeClassMap::BatchClassId));
513+
514+
Sci->FreeListInfo.BlockList.push_front(BG);
515+
}
516+
517+
if (UNLIKELY(Size == 0))
518+
return;
519+
520+
// This happens under 2 cases.
521+
// 1. just allocated a new `BatchGroup`.
522+
// 2. Only 1 block is pushed when the freelist is empty.
523+
if (BG->Batches.empty()) {
524+
// Construct the `TransferBatch` on the last element.
525+
TransferBatch *TB = reinterpret_cast<TransferBatch *>(
526+
decompactPtr(SizeClassMap::BatchClassId, Array[Size - 1]));
527+
TB->clear();
528+
// As mentioned above, addresses of `TransferBatch` and `BatchGroup` are
529+
// recorded in the TransferBatch.
530+
TB->add(Array[Size - 1]);
531+
TB->add(
532+
compactPtr(SizeClassMap::BatchClassId, reinterpret_cast<uptr>(BG)));
533+
--Size;
534+
DCHECK_EQ(BG->PushedBlocks, 1U);
535+
// `TB` is also the block of BatchClassId.
536+
BG->PushedBlocks += 1;
537+
BG->Batches.push_front(TB);
538+
}
539+
540+
TransferBatch *CurBatch = BG->Batches.front();
541+
DCHECK_NE(CurBatch, nullptr);
542+
543+
for (u32 I = 0; I < Size;) {
544+
u16 UnusedSlots =
545+
static_cast<u16>(BG->MaxCachedPerBatch - CurBatch->getCount());
546+
if (UnusedSlots == 0) {
547+
CurBatch = reinterpret_cast<TransferBatch *>(
548+
decompactPtr(SizeClassMap::BatchClassId, Array[I]));
549+
CurBatch->clear();
550+
// Self-contained
551+
CurBatch->add(Array[I]);
552+
++I;
553+
// TODO(chiahungduan): Avoid the use of push_back() in `Batches` of
554+
// BatchClassId.
555+
BG->Batches.push_front(CurBatch);
556+
UnusedSlots = BG->MaxCachedPerBatch - 1;
557+
}
558+
// `UnusedSlots` is u16 so the result will be also fit in u16.
559+
const u16 AppendSize = static_cast<u16>(Min<u32>(UnusedSlots, Size - I));
560+
CurBatch->appendFromArray(&Array[I], AppendSize);
561+
I += AppendSize;
562+
}
563+
564+
BG->PushedBlocks += Size;
565+
}
453566
// Push the blocks to their batch group. The layout will be like,
454567
//
455568
// FreeListInfo.BlockList - > BG -> BG -> BG
@@ -471,69 +584,16 @@ template <typename Config> class SizeClassAllocator32 {
471584
void pushBlocksImpl(CacheT *C, uptr ClassId, SizeClassInfo *Sci,
472585
CompactPtrT *Array, u32 Size, bool SameGroup = false)
473586
REQUIRES(Sci->Mutex) {
587+
DCHECK_NE(ClassId, SizeClassMap::BatchClassId);
474588
DCHECK_GT(Size, 0U);
475589

476590
auto CreateGroup = [&](uptr CompactPtrGroupBase) {
477-
BatchGroup *BG = nullptr;
478-
TransferBatch *TB = nullptr;
479-
if (ClassId == SizeClassMap::BatchClassId) {
480-
DCHECK_GE(Size, 2U);
481-
482-
// Free blocks are recorded by TransferBatch in freelist, blocks of
483-
// BatchClassId are included. In order not to use additional memory to
484-
// record blocks of BatchClassId, they are self-contained. I.e., A
485-
// TransferBatch may record the block address of itself. See the figure
486-
// below:
487-
//
488-
// TransferBatch at 0xABCD
489-
// +----------------------------+
490-
// | Free blocks' addr |
491-
// | +------+------+------+ |
492-
// | |0xABCD|... |... | |
493-
// | +------+------+------+ |
494-
// +----------------------------+
495-
//
496-
// The safeness of manipulating TransferBatch is kept by the invariant,
497-
//
498-
// The unit of each pop-block request is a TransferBatch. Return
499-
// part of the blocks in a TransferBatch is not allowed.
500-
//
501-
// This ensures that TransferBatch won't leak the address itself while
502-
// it's still holding other valid data.
503-
//
504-
// Besides, BatchGroup uses the same size-class as TransferBatch does
505-
// and its address is recorded in the TransferBatch too. To maintain the
506-
// safeness, the invariant to keep is,
507-
//
508-
// The address of itself is always recorded in the last TransferBatch
509-
// of the freelist (also imply that the freelist should only be
510-
// updated with push_front). Once the last TransferBatch is popped,
511-
// the BatchGroup becomes invalid.
512-
//
513-
// As a result, the blocks used by BatchGroup and TransferBatch are
514-
// reusable and don't need additional space for them.
515-
BG = reinterpret_cast<BatchGroup *>(
516-
decompactPtr(ClassId, Array[Size - 1]));
517-
BG->Batches.clear();
518-
519-
TB = reinterpret_cast<TransferBatch *>(
520-
decompactPtr(ClassId, Array[Size - 2]));
521-
TB->clear();
522-
523-
// Append the blocks used by BatchGroup and TransferBatch immediately so
524-
// that we ensure that they are in the last TransBatch.
525-
TB->appendFromArray(Array + Size - 2, 2);
526-
Size -= 2;
527-
} else {
528-
BG = C->createGroup();
529-
BG->Batches.clear();
530-
531-
TB = C->createBatch(ClassId, nullptr);
532-
TB->clear();
533-
}
591+
BatchGroup *BG = C->createGroup();
592+
BG->Batches.clear();
593+
TransferBatch *TB = C->createBatch(ClassId, nullptr);
594+
TB->clear();
534595

535596
BG->CompactPtrGroupBase = CompactPtrGroupBase;
536-
// TODO(chiahungduan): Avoid the use of push_back() in `Batches`.
537597
BG->Batches.push_front(TB);
538598
BG->PushedBlocks = 0;
539599
BG->BytesInBGAtLastCheckpoint = 0;
@@ -572,16 +632,6 @@ template <typename Config> class SizeClassAllocator32 {
572632
Sci->FreeListInfo.PushedBlocks += Size;
573633
BatchGroup *Cur = Sci->FreeListInfo.BlockList.front();
574634

575-
if (ClassId == SizeClassMap::BatchClassId) {
576-
if (Cur == nullptr) {
577-
// Don't need to classify BatchClassId.
578-
Cur = CreateGroup(/*CompactPtrGroupBase=*/0);
579-
Sci->FreeListInfo.BlockList.push_front(Cur);
580-
}
581-
InsertBlocks(Cur, Array, Size);
582-
return;
583-
}
584-
585635
// In the following, `Cur` always points to the BatchGroup for blocks that
586636
// will be pushed next. `Prev` is the element right before `Cur`.
587637
BatchGroup *Prev = nullptr;
@@ -652,7 +702,21 @@ template <typename Config> class SizeClassAllocator32 {
652702

653703
SinglyLinkedList<TransferBatch> &Batches =
654704
Sci->FreeListInfo.BlockList.front()->Batches;
655-
DCHECK(!Batches.empty());
705+
706+
if (Batches.empty()) {
707+
DCHECK_EQ(ClassId, SizeClassMap::BatchClassId);
708+
BatchGroup *BG = Sci->FreeListInfo.BlockList.front();
709+
Sci->FreeListInfo.BlockList.pop_front();
710+
711+
// Block used by `BatchGroup` is from BatchClassId. Turn the block into
712+
// `TransferBatch` with single block.
713+
TransferBatch *TB = reinterpret_cast<TransferBatch *>(BG);
714+
TB->clear();
715+
TB->add(
716+
compactPtr(SizeClassMap::BatchClassId, reinterpret_cast<uptr>(TB)));
717+
Sci->FreeListInfo.PoppedBlocks += 1;
718+
return TB;
719+
}
656720

657721
TransferBatch *B = Batches.front();
658722
Batches.pop_front();
@@ -742,8 +806,7 @@ template <typename Config> class SizeClassAllocator32 {
742806
pushBlocksImpl(C, ClassId, Sci, &ShuffleArray[NumberOfBlocks - N], N,
743807
/*SameGroup=*/true);
744808
} else {
745-
pushBlocksImpl(C, ClassId, Sci, ShuffleArray, NumberOfBlocks,
746-
/*SameGroup=*/true);
809+
pushBatchClassBlocks(Sci, ShuffleArray, NumberOfBlocks);
747810
}
748811

749812
// Note that `PushedBlocks` and `PoppedBlocks` are supposed to only record

0 commit comments

Comments
 (0)