diff --git a/src/gpgmm/common/SegmentedMemoryAllocator.cpp b/src/gpgmm/common/SegmentedMemoryAllocator.cpp index eb017db4b..a803963e8 100644 --- a/src/gpgmm/common/SegmentedMemoryAllocator.cpp +++ b/src/gpgmm/common/SegmentedMemoryAllocator.cpp @@ -96,7 +96,7 @@ namespace gpgmm { if (existingFreeSegment == mFreeSegments.end()) { ASSERT(mFreeSegments.empty()); MemorySegment* newFreeSegment = new MemorySegment{memorySize}; - mFreeSegments.push_back(newFreeSegment); + newFreeSegment->InsertAfter(mFreeSegments.tail()); return newFreeSegment; } diff --git a/src/gpgmm/common/SlabMemoryAllocator.cpp b/src/gpgmm/common/SlabMemoryAllocator.cpp index c14cbbde8..6a99b45ad 100644 --- a/src/gpgmm/common/SlabMemoryAllocator.cpp +++ b/src/gpgmm/common/SlabMemoryAllocator.cpp @@ -318,6 +318,13 @@ namespace gpgmm { } } + // If the slab is now full, move it to the full-list so it does not remain the free-list + // where de-allocate could mistakenly remove it from the wrong list. + if (pFreeSlab->IsFull()) { + pCache->FreeList.remove(pFreeSlab); + pCache->FullList.push_front(pFreeSlab); + } + // Wrap the block in the containing slab. Since the slab's block could reside in another // allocated block, the slab's allocation offset must be made relative to slab's underlying // memory and not the slab. @@ -347,12 +354,9 @@ namespace gpgmm { Slab* slab = blockInSlab->pSlab; ASSERT(slab != nullptr); - MemoryBase* slabMemory = subAllocation->GetMemory(); - ASSERT(slabMemory != nullptr); - // Splice the slab from the full-list to free-list. if (slab->IsFull()) { - SlabCache* cache = GetOrCreateCache(slabMemory->GetSize()); + SlabCache* cache = GetOrCreateCache(slab->SlabMemory->GetSize()); cache->FullList.remove(slab); cache->FreeList.push_front(slab); } @@ -364,6 +368,9 @@ namespace gpgmm { slab->Allocator.DeallocateBlock(block); SafeDelete(blockInSlab); + MemoryBase* slabMemory = subAllocation->GetMemory(); + ASSERT(slabMemory != nullptr); + slabMemory->Unref(); // If the slab will be empty, release the underlying memory. @@ -448,7 +455,7 @@ namespace gpgmm { blockSize, mMaxSlabSize, mMinSlabSize, mSlabAlignment, mSlabFragmentationLimit, mAllowSlabPrefetch, mSlabGrowthFactor, GetNextInChain()); entry->GetValue().pSlabAllocator = slabAllocator; - mSlabAllocators.push_back(slabAllocator); + slabAllocator->InsertAfter(mSlabAllocators.tail()); } ASSERT(slabAllocator != nullptr); diff --git a/src/gpgmm/common/SlabMemoryAllocator.h b/src/gpgmm/common/SlabMemoryAllocator.h index 3623d39d5..9bdcfa0f6 100644 --- a/src/gpgmm/common/SlabMemoryAllocator.h +++ b/src/gpgmm/common/SlabMemoryAllocator.h @@ -112,10 +112,10 @@ namespace gpgmm { // Group of one or more slabs of the same size. struct SlabCache { - LinkedList FreeList; // Slabs that contain partial or empty - // slabs or some free blocks. - LinkedList FullList; // Slabs that are full or all blocks - // are marked as used. + SizedLinkedList FreeList; // Slabs that contain partial or empty + // slabs or some free blocks. + SizedLinkedList FullList; // Slabs that are full or all blocks + // are marked as used. }; SlabCache* GetOrCreateCache(uint64_t slabSize); diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp index 46f0c9bd2..38a2a6437 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp @@ -189,7 +189,7 @@ namespace gpgmm::d3d12 { LRUCache* cache = GetVideoMemorySegmentCache(heap->GetMemorySegmentGroup()); ASSERT(cache != nullptr); - cache->push_back(heap); + heap->InsertAfter(cache->tail()); ASSERT(heap->IsInList()); diff --git a/src/gpgmm/utils/LinkedList.h b/src/gpgmm/utils/LinkedList.h index 50c1cb62c..612640c3b 100644 --- a/src/gpgmm/utils/LinkedList.h +++ b/src/gpgmm/utils/LinkedList.h @@ -227,32 +227,7 @@ namespace gpgmm { // Using LinkedList in std::vector or STL container requires the move constructor to not // throw. - LinkedList(LinkedList&& other) noexcept - : root_(std::move(other.root_)), size_(other.size_) { - other.size_ = 0; - } - - // Appends |e| to the end of the linked list. - void push_back(LinkNode* e) { - e->InsertBefore(&root_); - size_++; - } - - // Prepend |e| to the start of the linked list. - void push_front(LinkNode* e) { - e->InsertBefore(head()); - size_++; - } - - // Removes the first node in the linked list. - void pop_front() { - remove(head()); - } - - // Removes |e| from the linked list, decreasing the size by 1. - void remove(LinkNode* e) { - e->RemoveFromList(); - size_--; + LinkedList(LinkedList&& other) noexcept : root_(std::move(other.root_)) { } LinkNode* head() const { @@ -275,11 +250,6 @@ namespace gpgmm { SafeDelete(curr.value()); } ASSERT(empty()); - size_ = 0; - } - - size_t size() const { - return size_; } iterator begin() { @@ -300,6 +270,48 @@ namespace gpgmm { private: LinkNode root_; + }; + + // SizedLinkedList is like LinkedList but also keeps track of the number of nodes in the + // list. Random insertion is not supported, only deletion. To insert, push from start or end + // of the list. + template + class SizedLinkedList : public LinkedList { + public: + // Appends |e| to the end of the linked list. + void push_back(LinkNode* e) { + e->InsertBefore(LinkedList::end()); + size_++; + } + + // Prepend |e| to the start of the linked list. + void push_front(LinkNode* e) { + e->InsertBefore(LinkedList::head()); + size_++; + } + + // Removes the first node in the linked list. + void pop_front() { + remove(LinkedList::head()); + } + + // Removes |e| from the linked list, decreasing the size by 1. + void remove(LinkNode* e) { + ASSERT(size_ > 0); + e->RemoveFromList(); + size_--; + } + + size_t size() const { + return size_; + } + + void clear() { + LinkedList::clear(); + size_ = 0; + } + + private: size_t size_ = 0; }; diff --git a/src/tests/unittests/LinkedListTests.cpp b/src/tests/unittests/LinkedListTests.cpp index 7a4645f38..fa2d3cc24 100644 --- a/src/tests/unittests/LinkedListTests.cpp +++ b/src/tests/unittests/LinkedListTests.cpp @@ -41,15 +41,14 @@ TEST_F(LinkedListTests, Insert) { LinkNode* end = new FakeObject(); LinkedList list; - list.push_front(middle); - list.push_front(start); - list.push_back(end); + start->InsertAfter(list.tail()); + middle->InsertAfter(list.tail()); + end->InsertAfter(list.tail()); EXPECT_EQ(list.head(), start); EXPECT_EQ(list.tail(), end); EXPECT_FALSE(list.empty()); - EXPECT_EQ(list.size(), 3u); } TEST_F(LinkedListTests, Remove) { @@ -58,16 +57,15 @@ TEST_F(LinkedListTests, Remove) { LinkNode* end = new FakeObject(); LinkedList list; - list.push_back(start); - list.push_back(middle); - list.push_back(end); + start->InsertAfter(list.tail()); + middle->InsertAfter(list.tail()); + end->InsertAfter(list.tail()); - list.remove(middle); - list.remove(start); - list.remove(end); + start->RemoveFromList(); + middle->RemoveFromList(); + end->RemoveFromList(); EXPECT_TRUE(list.empty()); - EXPECT_EQ(list.size(), 0u); } TEST_F(LinkedListTests, Clear) { @@ -76,31 +74,26 @@ TEST_F(LinkedListTests, Clear) { LinkNode* third = new FakeObject(); LinkedList list; - list.push_back(first); - list.push_back(second); - list.push_back(third); - - EXPECT_EQ(list.size(), 3u); + first->InsertAfter(list.tail()); + second->InsertAfter(list.tail()); + third->InsertAfter(list.tail()); list.clear(); EXPECT_TRUE(list.empty()); - EXPECT_EQ(list.size(), 0u); } TEST_F(LinkedListTests, Move) { LinkNode* objectInFirstList = new FakeObject(); LinkedList firstList; - firstList.push_back(objectInFirstList); - EXPECT_EQ(firstList.size(), 1u); + objectInFirstList->InsertAfter(firstList.tail()); EXPECT_FALSE(firstList.empty()); EXPECT_EQ(firstList.head(), objectInFirstList); EXPECT_EQ(firstList.tail(), objectInFirstList); LinkedList secondList(std::move(firstList)); - EXPECT_EQ(secondList.size(), 1u); EXPECT_FALSE(secondList.empty()); EXPECT_EQ(secondList.head(), objectInFirstList); @@ -108,10 +101,14 @@ TEST_F(LinkedListTests, Move) { } TEST_F(LinkedListTests, Iterator) { + LinkNode* first = new FakeObject(1); + LinkNode* second = new FakeObject(2); + LinkNode* third = new FakeObject(3); + LinkedList list; - list.push_back(new FakeObject(1)); - list.push_back(new FakeObject(2)); - list.push_back(new FakeObject(3)); + first->InsertAfter(list.tail()); + second->InsertAfter(list.tail()); + third->InsertAfter(list.tail()); // Iterate through the whole range. size_t index = 0; @@ -119,22 +116,42 @@ TEST_F(LinkedListTests, Iterator) { EXPECT_EQ(node.value()->mId, ++index); } - EXPECT_EQ(index, list.size()); - // Iterate through the whole range again (but using const). index = 0; for (const auto& node : list) { EXPECT_EQ(node.value()->mId, ++index); } - // Iterate through the whole range but remove the first. + // Iterate through the whole range but have the first remove itself. index = 0; for (auto& node : list) { if (node.value()->mId == 1) { - list.remove(node.value()); + node.RemoveFromList(); } EXPECT_EQ(node.value()->mId, ++index); } +} - EXPECT_EQ(index, list.size() + 1); +TEST_F(LinkedListTests, ListWithSize) { + LinkNode* first = new FakeObject(1); + LinkNode* second = new FakeObject(2); + LinkNode* third = new FakeObject(3); + + SizedLinkedList list; + list.push_front(first); + list.push_front(second); + list.push_front(third); + + EXPECT_EQ(list.size(), 3u); + + list.pop_front(); + EXPECT_EQ(list.head(), second); + EXPECT_EQ(list.size(), 2u); + + list.remove(second); + EXPECT_EQ(list.head(), first); + EXPECT_EQ(list.size(), 1u); + + list.clear(); + EXPECT_EQ(list.size(), 0u); }