Skip to content

Commit e1c0f9b

Browse files
committed
perf: avoid arena lock for miniheap reuse fast path
Change lock ordering from arena->size-class to size-class->arena. This allows allocSmallMiniheaps() to only acquire the size-class lock when reusing existing miniheaps from freelists, deferring arena lock acquisition to the slow path (allocating new miniheaps). Previously, every call to allocSmallMiniheaps() grabbed the arena lock even though the common case (reusing miniheaps from partial/empty freelists) never touches arena state. The arena lock protects: - _mhAllocator (miniheap metadata allocator) - pageAlloc() (allocating pages from arena) - trackMiniHeap() (updating page-to-miniheap mapping) None of these are needed when simply taking a miniheap from a freelist and marking it as attached. Updated all lock acquisition sites for consistency: - AllLocksGuard: size-classes[0..N-1] -> large -> arena - allocSmallMiniheaps: size-class lock first, arena only if needed - pageAlignedAlloc: large lock -> arena lock - freeMiniheap: size-class/large lock -> arena lock - freeFor (large): large lock -> arena lock - lock()/unlock(): include arena lock for fork handling
1 parent 7b8aace commit e1c0f9b

File tree

2 files changed

+34
-27
lines changed

2 files changed

+34
-27
lines changed

src/global_heap.h

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -90,21 +90,23 @@ class GlobalHeap : public MeshableArena {
9090
public:
9191
AllLocksGuard(std::array<mutex, kNumBins> &locks, mutex &largeLock, mutex &arenaLockRef)
9292
: _locks(locks), _largeLock(largeLock), _arenaLockRef(arenaLockRef) {
93-
// Always acquire in consistent order: arena first, large lock, then size classes 0..N-1
94-
_arenaLockRef.lock();
95-
_largeLock.lock();
93+
// Lock ordering: size-classes[0..N-1] -> large -> arena
94+
// This allows the fast path (reusing miniheaps) to only acquire size-class lock,
95+
// then optionally acquire arena lock later if new allocation is needed.
9696
for (size_t i = 0; i < kNumBins; i++) {
9797
_locks[i].lock();
9898
}
99+
_largeLock.lock();
100+
_arenaLockRef.lock();
99101
}
100102

101103
~AllLocksGuard() {
102104
// Release in reverse order
105+
_arenaLockRef.unlock();
106+
_largeLock.unlock();
103107
for (size_t i = kNumBins; i > 0; i--) {
104108
_locks[i - 1].unlock();
105109
}
106-
_largeLock.unlock();
107-
_arenaLockRef.unlock();
108110
}
109111
};
110112

@@ -171,9 +173,9 @@ class GlobalHeap : public MeshableArena {
171173
return nullptr;
172174
}
173175

174-
// Lock ordering: arena lock first, then large alloc lock
175-
lock_guard<mutex> arenaLock(_arenaLock);
176+
// Lock ordering: large alloc lock -> arena lock
176177
lock_guard<mutex> lock(_largeAllocLock);
178+
lock_guard<mutex> arenaLock(_arenaLock);
177179

178180
const size_t pageSize = getPageSize();
179181
MiniHeapT *mh = allocMiniheapLocked(-1, pageCount, 1, pageCount * pageSize, pageAlignment);
@@ -391,9 +393,17 @@ class GlobalHeap : public MeshableArena {
391393
pid_t current) {
392394
d_assert(sizeClass >= 0);
393395
d_assert(sizeClass < kNumBins);
396+
d_assert(objectSize <= _maxObjectSize);
394397

395-
// Lock ordering: arena lock first, then size-class lock
396-
lock_guard<mutex> arenaLock(_arenaLock);
398+
#ifndef NDEBUG
399+
const size_t classMaxSize = SizeMap::ByteSizeForClass(sizeClass);
400+
d_assert_msg(objectSize == classMaxSize, "sz(%zu) shouldn't be greater than %zu (class %d)", objectSize,
401+
classMaxSize, sizeClass);
402+
#endif
403+
404+
// Lock ordering: size-class lock -> arena lock
405+
// We acquire size-class lock first and try to reuse existing miniheaps.
406+
// Only if we need new miniheaps do we acquire the arena lock.
397407
lock_guard<mutex> lock(_miniheapLocks[sizeClass]);
398408

399409
// Drain pending partial list so freed miniheaps are immediately available
@@ -404,23 +414,17 @@ class GlobalHeap : public MeshableArena {
404414
}
405415
miniheaps.clear();
406416

407-
d_assert(objectSize <= _maxObjectSize);
408-
409-
#ifndef NDEBUG
410-
const size_t classMaxSize = SizeMap::ByteSizeForClass(sizeClass);
411-
412-
d_assert_msg(objectSize == classMaxSize, "sz(%zu) shouldn't be greater than %zu (class %d)", objectSize,
413-
classMaxSize, sizeClass);
414-
#endif
415-
416417
d_assert(miniheaps.size() == 0);
417418

418-
// check our bins for a miniheap to reuse
419+
// Fast path: check our bins for a miniheap to reuse (no arena lock needed)
419420
auto bytesFree = selectForReuse(sizeClass, miniheaps, current);
420421
if (bytesFree >= kMiniheapRefillGoalSize || miniheaps.full()) {
421422
return;
422423
}
423424

425+
// Slow path: need to allocate new miniheaps, acquire arena lock
426+
lock_guard<mutex> arenaLock(_arenaLock);
427+
424428
// if we have objects bigger than the size of a page, allocate
425429
// multiple pages to amortize the cost of creating a
426430
// miniheap/globally locking the heap. For example, asking for
@@ -491,14 +495,15 @@ class GlobalHeap : public MeshableArena {
491495

492496
void freeMiniheap(MiniHeapT *&mh, bool untrack = true) {
493497
const int sizeClass = mh->sizeClass();
494-
// Lock ordering: arena lock first, then size-class/large lock
495-
lock_guard<mutex> arenaLock(_arenaLock);
498+
// Lock ordering: size-class/large lock -> arena lock
496499
if (sizeClass >= 0) {
497500
lock_guard<mutex> lock(_miniheapLocks[sizeClass]);
501+
lock_guard<mutex> arenaLock(_arenaLock);
498502
freeMiniheapLocked(mh, untrack);
499503
} else {
500504
// Large allocation
501505
lock_guard<mutex> lock(_largeAllocLock);
506+
lock_guard<mutex> arenaLock(_arenaLock);
502507
freeMiniheapLocked(mh, untrack);
503508
}
504509
}
@@ -593,19 +598,21 @@ class GlobalHeap : public MeshableArena {
593598
}
594599

595600
void lock() {
596-
// Acquire all locks in consistent order
597-
_largeAllocLock.lock();
601+
// Acquire all locks in consistent order: size-classes -> large -> arena
598602
for (size_t i = 0; i < kNumBins; i++) {
599603
_miniheapLocks[i].lock();
600604
}
605+
_largeAllocLock.lock();
606+
_arenaLock.lock();
601607
}
602608

603609
void unlock() {
604-
// Release all locks in reverse order
610+
// Release in reverse order
611+
_arenaLock.unlock();
612+
_largeAllocLock.unlock();
605613
for (size_t i = kNumBins; i > 0; i--) {
606614
_miniheapLocks[i - 1].unlock();
607615
}
608-
_largeAllocLock.unlock();
609616
}
610617

611618
// PUBLIC ONLY FOR TESTING

src/global_heap_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ void GlobalHeap<PageSize>::freeFor(MiniHeapT *mh, void *ptr, size_t startEpoch)
9999
// This can also include, for example, single page allocations w/
100100
// 16KB alignment.
101101
if (mh->isLargeAlloc()) {
102-
// Lock ordering: arena lock first, then large alloc lock
103-
lock_guard<mutex> arenaLock(_arenaLock);
102+
// Lock ordering: large alloc lock -> arena lock
104103
lock_guard<mutex> lock(_largeAllocLock);
104+
lock_guard<mutex> arenaLock(_arenaLock);
105105
freeMiniheapLocked(mh, false);
106106
return;
107107
}

0 commit comments

Comments
 (0)