diff --git a/src/gpgmm/common/BlockAllocator.h b/src/gpgmm/common/BlockAllocator.h index 08635182..9c56122c 100644 --- a/src/gpgmm/common/BlockAllocator.h +++ b/src/gpgmm/common/BlockAllocator.h @@ -15,6 +15,7 @@ #ifndef SRC_GPGMM_COMMON_BLOCKALLOCATOR_H_ #define SRC_GPGMM_COMMON_BLOCKALLOCATOR_H_ +#include "gpgmm/common/Error.h" #include "gpgmm/common/MemoryBlock.h" #include "gpgmm/common/Object.h" #include "gpgmm/utils/NonCopyable.h" @@ -26,7 +27,8 @@ namespace gpgmm { public: ~BlockAllocator() override = default; - virtual MemoryBlock* TryAllocateBlock(uint64_t requestSize, uint64_t alignment) = 0; + virtual ResultOrError TryAllocateBlock(uint64_t requestSize, + uint64_t alignment) = 0; virtual void DeallocateBlock(MemoryBlock* block) = 0; }; diff --git a/src/gpgmm/common/BuddyBlockAllocator.cpp b/src/gpgmm/common/BuddyBlockAllocator.cpp index 14d7dd4d..2c5107ca 100644 --- a/src/gpgmm/common/BuddyBlockAllocator.cpp +++ b/src/gpgmm/common/BuddyBlockAllocator.cpp @@ -144,10 +144,11 @@ namespace gpgmm { } } - MemoryBlock* BuddyBlockAllocator::TryAllocateBlock(uint64_t requestSize, uint64_t alignment) { + ResultOrError BuddyBlockAllocator::TryAllocateBlock(uint64_t requestSize, + uint64_t alignment) { // Request cannot exceed max block size. if (requestSize > mMaxBlockSize) { - return nullptr; + return {}; } // Compute the level @@ -159,7 +160,7 @@ namespace gpgmm { // Error when no free blocks exist (allocator is full) if (currBlockLevel == kInvalidOffset) { - return nullptr; + return {}; } // Split free blocks level-by-level. diff --git a/src/gpgmm/common/BuddyBlockAllocator.h b/src/gpgmm/common/BuddyBlockAllocator.h index 644b704c..14d5614f 100644 --- a/src/gpgmm/common/BuddyBlockAllocator.h +++ b/src/gpgmm/common/BuddyBlockAllocator.h @@ -42,7 +42,8 @@ namespace gpgmm { ~BuddyBlockAllocator() override; // BlockAllocator interface - MemoryBlock* TryAllocateBlock(uint64_t requestSize, uint64_t alignment) override; + ResultOrError TryAllocateBlock(uint64_t requestSize, + uint64_t alignment) override; void DeallocateBlock(MemoryBlock* block) override; // For testing purposes only. diff --git a/src/gpgmm/common/MemoryAllocator.h b/src/gpgmm/common/MemoryAllocator.h index 335d8478..bf8559b8 100644 --- a/src/gpgmm/common/MemoryAllocator.h +++ b/src/gpgmm/common/MemoryAllocator.h @@ -225,13 +225,15 @@ namespace gpgmm { uint64_t alignment, bool neverAllocate, GetOrCreateMemoryFn&& GetOrCreateMemory) { - MemoryBlock* block = allocator->TryAllocateBlock(requestSize, alignment); - if (block == nullptr) { - return {}; + ResultOrError allocatedBlockResult = + allocator->TryAllocateBlock(requestSize, alignment); + if (!allocatedBlockResult.IsSuccess()) { + return allocatedBlockResult.GetErrorCode(); } - - ResultOrError> result = GetOrCreateMemory(block); - if (!result.IsSuccess()) { + MemoryBlock* block = allocatedBlockResult.AcquireResult(); + ResultOrError> allocatedMemoryResult = + GetOrCreateMemory(block); + if (!allocatedMemoryResult.IsSuccess()) { // NeverAllocate always fails, so suppress it. if (!neverAllocate) { ErrorLog(ErrorCode::kAllocationFailed, this) @@ -240,10 +242,11 @@ namespace gpgmm { << std::to_string(block->Offset + block->Size) << ")."; } allocator->DeallocateBlock(block); - return result.GetErrorCode(); + return allocatedMemoryResult.GetErrorCode(); } - std::unique_ptr memoryAllocation = result.AcquireResult(); + std::unique_ptr memoryAllocation = + allocatedMemoryResult.AcquireResult(); ASSERT(memoryAllocation->GetMemory() != nullptr); memoryAllocation->GetMemory()->Ref(); diff --git a/src/gpgmm/common/SlabBlockAllocator.cpp b/src/gpgmm/common/SlabBlockAllocator.cpp index ae6454cb..bb6595e9 100644 --- a/src/gpgmm/common/SlabBlockAllocator.cpp +++ b/src/gpgmm/common/SlabBlockAllocator.cpp @@ -65,15 +65,16 @@ namespace gpgmm { mFreeList.pHead = nullptr; } - MemoryBlock* SlabBlockAllocator::TryAllocateBlock(uint64_t requestSize, uint64_t alignment) { + ResultOrError SlabBlockAllocator::TryAllocateBlock(uint64_t requestSize, + uint64_t alignment) { // Requested cannot exceed block size. if (requestSize > mBlockSize) { - return nullptr; + return {}; } // Offset must be equal to a multiple of |mBlockSize|. if (!IsAligned(mBlockSize, alignment)) { - return nullptr; + return {}; } // Pop off HEAD in the free-list. diff --git a/src/gpgmm/common/SlabBlockAllocator.h b/src/gpgmm/common/SlabBlockAllocator.h index 4d8eaa6c..24db62fb 100644 --- a/src/gpgmm/common/SlabBlockAllocator.h +++ b/src/gpgmm/common/SlabBlockAllocator.h @@ -47,7 +47,8 @@ namespace gpgmm { void ReleaseBlocks(); // BlockAllocator interface - MemoryBlock* TryAllocateBlock(uint64_t requestSize, uint64_t alignment = 1) override; + ResultOrError TryAllocateBlock(uint64_t requestSize, + uint64_t alignment) override; void DeallocateBlock(MemoryBlock* block) override; uint64_t GetBlockCount() const; diff --git a/src/tests/unittests/BuddyBlockAllocatorTests.cpp b/src/tests/unittests/BuddyBlockAllocatorTests.cpp index 680e733e..40319b9e 100644 --- a/src/tests/unittests/BuddyBlockAllocatorTests.cpp +++ b/src/tests/unittests/BuddyBlockAllocatorTests.cpp @@ -25,7 +25,7 @@ class DummyBuddyBlockAllocator { } MemoryBlock* TryAllocateBlock(uint64_t requestSize, uint64_t alignment = 1) { - return mAllocator.TryAllocateBlock(requestSize, alignment); + return mAllocator.TryAllocateBlock(requestSize, alignment).AcquireResult(); } void DeallocateBlock(MemoryBlock* block) { diff --git a/src/tests/unittests/SlabBlockAllocatorTests.cpp b/src/tests/unittests/SlabBlockAllocatorTests.cpp index 8aeb9e70..f4670118 100644 --- a/src/tests/unittests/SlabBlockAllocatorTests.cpp +++ b/src/tests/unittests/SlabBlockAllocatorTests.cpp @@ -21,11 +21,28 @@ using namespace gpgmm; +class DummySlabBlockAllocator { + public: + DummySlabBlockAllocator(uint64_t blockCount, uint64_t blockSize) + : mAllocator(blockCount, blockSize) { + } + + MemoryBlock* TryAllocateBlock(uint64_t requestSize, uint64_t alignment = 1) { + return mAllocator.TryAllocateBlock(requestSize, alignment).AcquireResult(); + } + + void DeallocateBlock(MemoryBlock* block) { + mAllocator.DeallocateBlock(block); + } + + SlabBlockAllocator mAllocator; +}; + // Verify a single allocation in a slab. TEST(SlabBlockAllocatorTests, SingleBlock) { constexpr uint64_t blockSize = 32; constexpr uint64_t slabSize = 128; - SlabBlockAllocator allocator(slabSize / blockSize, blockSize); + DummySlabBlockAllocator allocator(slabSize / blockSize, blockSize); // Check that we cannot allocate a oversized block. EXPECT_EQ(allocator.TryAllocateBlock(slabSize * 2), nullptr); @@ -45,7 +62,7 @@ TEST(SlabBlockAllocatorTests, SingleBlock) { TEST(SlabBlockAllocatorTests, SingleBlockAligned) { constexpr uint64_t blockSize = 32; constexpr uint64_t slabSize = 128; - SlabBlockAllocator allocator(slabSize / blockSize, blockSize); + DummySlabBlockAllocator allocator(slabSize / blockSize, blockSize); // Check that we cannot allocate a misaligned block. EXPECT_EQ(allocator.TryAllocateBlock(blockSize, 64u), nullptr); @@ -62,7 +79,7 @@ TEST(SlabBlockAllocatorTests, MultipleBlocks) { // Fill entire slab in the allocator. constexpr uint64_t blockSize = 32; constexpr uint64_t slabSize = 128; - SlabBlockAllocator allocator(slabSize / blockSize, blockSize); + DummySlabBlockAllocator allocator(slabSize / blockSize, blockSize); std::unordered_set blocks = {}; for (uint64_t blockIdx = 0; blockIdx < slabSize / blockSize; blockIdx++) { @@ -88,7 +105,7 @@ TEST(SlabBlockAllocatorTests, MultipleBlocksVarious) { // Fill entire slab in the allocator. constexpr uint64_t blockSize = 32; constexpr uint64_t slabSize = 128; - SlabBlockAllocator allocator(slabSize / blockSize, blockSize); + DummySlabBlockAllocator allocator(slabSize / blockSize, blockSize); std::unordered_set blocks = {};