Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
#5893: Improve block merge algorithm, expand unit tests
  • Loading branch information
codereader committed Jan 30, 2022
1 parent 6a68cce commit 908f01f
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 28 deletions.
70 changes: 57 additions & 13 deletions libs/render/ContinuousBuffer.h
Expand Up @@ -92,28 +92,72 @@ class ContinuousBuffer
releasedSlot.Occupied = false;

// Check if the slot can merge with an adjacent one
if (handle > 0 && !_slots[handle - 1].Occupied)
Handle slotIndexToMerge = std::numeric_limits<Handle>::max();
if (findLeftFreeSlot(releasedSlot, slotIndexToMerge))
{
// Merge the slot before the released one
_slots[handle - 1].Size += releasedSlot.Size;

// The released handle goes to waste, block it
releasedSlot.Occupied = true;
releasedSlot.Size = 0;
auto& slotToMerge = _slots[slotIndexToMerge];

releasedSlot.Offset = slotToMerge.Offset;
releasedSlot.Size += slotToMerge.Size;

// The merged handle goes to waste, block it against future use
slotToMerge.Size = 0;
slotToMerge.Occupied = true;
}
else if (handle < _slots.size() - 1 && !_slots[handle + 1].Occupied)

// Try to find an adjacent free slot to the right
if (findRightFreeSlot(releasedSlot, slotIndexToMerge))
{
auto& nextSlot = _slots[handle + 1];
auto& slotToMerge = _slots[slotIndexToMerge];

releasedSlot.Size += nextSlot.Size;
releasedSlot.Size += slotToMerge.Size;

// The next slot will go to waste, never use that again
nextSlot.Occupied = true;
nextSlot.Size = 0;
// The merged handle goes to waste, block it against future use
slotToMerge.Size = 0;
slotToMerge.Occupied = true;
}
}

private:
bool findLeftFreeSlot(const SlotInfo& slotToTouch, Handle& found)
{
auto numSlots = _slots.size();

for (Handle slotIndex = 0; slotIndex < numSlots; ++slotIndex)
{
const auto& candidate = _slots[slotIndex];

if (candidate.Offset + candidate.Size == slotToTouch.Offset)
{
// The slot coordinates match, return true if this block is free
found = slotIndex;
return !candidate.Occupied;
}
}

return false;
}

bool findRightFreeSlot(const SlotInfo& slotToTouch, Handle& found)
{
auto numSlots = _slots.size();
auto offsetToMatch = slotToTouch.Offset + slotToTouch.Size;

for (Handle slotIndex = 0; slotIndex < numSlots; ++slotIndex)
{
const auto& candidate = _slots[slotIndex];

if (candidate.Offset == offsetToMatch)
{
// The slot coordinates match, return true if this block is free
found = slotIndex;
return !candidate.Occupied;
}
}

return false;
}

Handle getNextFreeSlotForSize(std::size_t requiredSize)
{
auto numSlots = _slots.size();
Expand Down
135 changes: 120 additions & 15 deletions test/ContinuousBuffer.cpp
Expand Up @@ -21,6 +21,25 @@ bool checkData(render::ContinuousBuffer<T>& buffer, typename render::ContinuousB
return result;
}

template<typename T>
bool checkContinuousData(render::ContinuousBuffer<T>& buffer, typename render::ContinuousBuffer<T>::Handle handle,
const std::initializer_list<std::vector<T>>& dataVectors)
{
// Create a continuous buffer
std::vector<T> fullData;

for (const auto& data : dataVectors)
{
fullData.insert(fullData.end(), data.begin(), data.end());
}

auto result = checkData(buffer, handle, fullData);

EXPECT_TRUE(result) << "Data not continuously stored";

return result;
}

TEST(ContinuousBufferTest, InitialSize)
{
// Construct a buffer of a certain initial size
Expand Down Expand Up @@ -56,7 +75,22 @@ TEST(ContinuousBufferTest, AllocateAndDeallocate)

EXPECT_EQ(buffer.getOffset(handle), 0) << "Data should be located at offset 0";
EXPECT_TRUE(checkData(buffer, handle, sixteen));
}

TEST(ContinuousBufferTest, ReplaceData)
{
auto eight = std::vector<int>({ 0,1,2,3,4,5,6,7 });
auto eightAlt = std::vector<int>({ 8,9,10,11,12,13,14,15 });

render::ContinuousBuffer<int> buffer(24);

auto handle = buffer.allocate(eight.size());
buffer.setData(handle, eight);

EXPECT_TRUE(checkData(buffer, handle, eight));

buffer.setData(handle, eightAlt);
EXPECT_TRUE(checkData(buffer, handle, eightAlt));
}

TEST(ContinuousBufferTest, BufferGrowth)
Expand Down Expand Up @@ -88,11 +122,7 @@ TEST(ContinuousBufferTest, BufferGrowth)
EXPECT_TRUE(checkData(buffer, handle3, eight)) << "Third data batch not stored correctly";

// Data should be continuously stored
auto fullData = sixteen;
fullData.insert(fullData.end(), eight.begin(), eight.end());
fullData.insert(fullData.end(), eight.begin(), eight.end());

EXPECT_TRUE(checkData(buffer, handle1, fullData)) << "Data not continuously stored";
EXPECT_TRUE(checkContinuousData(buffer, handle1, { sixteen, eight, eight }));

buffer.deallocate(handle1);
buffer.deallocate(handle2);
Expand All @@ -117,27 +147,102 @@ TEST(ContinuousBufferTest, BlockReuse)
buffer.setData(handle3, eight);

// Check that the whole data is continuous
auto fullData = sixteen;
fullData.insert(fullData.end(), eight.begin(), eight.end());
fullData.insert(fullData.end(), eight.begin(), eight.end());

EXPECT_TRUE(checkData(buffer, handle1, fullData)) << "Data not continuously stored";
EXPECT_TRUE(checkContinuousData(buffer, handle1, { sixteen, eight, eight }));

// Release one of the 8-length buffers and replace the data
buffer.deallocate(handle2);

handle2 = buffer.allocate(eightFrom6.size());

buffer.setData(handle2, eightFrom6);

fullData = sixteen;
fullData.insert(fullData.end(), eightFrom6.begin(), eightFrom6.end());
fullData.insert(fullData.end(), eight.begin(), eight.end());
EXPECT_TRUE(checkContinuousData(buffer, handle1, { sixteen, eightFrom6, eight }));

EXPECT_TRUE(checkData(buffer, handle1, fullData)) << "New data not continuously stored";
buffer.deallocate(handle1);
buffer.deallocate(handle2);
buffer.deallocate(handle3);
}

TEST(ContinuousBufferTest, BlockReuseTightlyFit)
{
auto eight = std::vector<int>({ 0,1,2,3,4,5,6,7 });
auto sixteen = std::vector<int>({ 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 });
auto four = std::vector<int>({ 10,11,12,13 });

render::ContinuousBuffer<int> buffer(18); // Allocate a buffer of size 18

auto handle1 = buffer.allocate(eight.size());
auto handle2 = buffer.allocate(sixteen.size());

buffer.setData(handle1, eight);
buffer.setData(handle2, sixteen);

EXPECT_TRUE(checkContinuousData(buffer, handle1, { eight, sixteen }));

// Release the 8-length buffers and replace the data with 2x 4
buffer.deallocate(handle1);

auto handle3 = buffer.allocate(four.size()); // this will leave a gap of 4
auto handle4 = buffer.allocate(eight.size()); // this one will not fit into the gap
auto handle5 = buffer.allocate(four.size()); // this one will

buffer.setData(handle3, four);
buffer.setData(handle4, eight);
buffer.setData(handle5, four);

EXPECT_TRUE(checkContinuousData(buffer, handle1, { four, four, sixteen, eight }));

buffer.deallocate(handle2);
buffer.deallocate(handle3);
buffer.deallocate(handle4);
buffer.deallocate(handle5);
}

TEST(ContinuousBufferTest, GapMerging)
{
auto eight = std::vector<int>({ 0,1,2,3,4,5,6,7 });
auto four = std::vector<int>({ 10,11,12,13 });
auto sixteen = std::vector<int>({ 20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35 });

render::ContinuousBuffer<int> buffer(18); // Allocate a buffer of size 18

auto handle1 = buffer.allocate(four.size());
auto handle2 = buffer.allocate(four.size());
auto handle3 = buffer.allocate(four.size());
auto handle4 = buffer.allocate(four.size());

buffer.setData(handle1, four);
buffer.setData(handle2, four);
buffer.setData(handle3, four);
buffer.setData(handle4, four);

EXPECT_TRUE(checkContinuousData(buffer, handle1, { four, four, four, four }));

// Release two continuous buffers (left one first) and try to fit in a 8-length data chunk
buffer.deallocate(handle1);
buffer.deallocate(handle2);

auto handle5 = buffer.allocate(eight.size()); // this will fit tightly into the 2x4 gap
buffer.setData(handle5, eight);

EXPECT_TRUE(checkContinuousData(buffer, handle5, { eight, four, four }));

// Release two more buffers (right one first) and try to fit in a 8-length data chunk
buffer.deallocate(handle4); // rightmost first
buffer.deallocate(handle3);

auto handle6 = buffer.allocate(eight.size()); // this will fit tightly into the 2x4 gap
buffer.setData(handle6, eight);

EXPECT_TRUE(checkContinuousData(buffer, handle1, { eight, eight }));

buffer.deallocate(handle5);
buffer.deallocate(handle6);

auto handle7 = buffer.allocate(sixteen.size()); // entire buffer is clear now
buffer.setData(handle7, sixteen);

EXPECT_EQ(buffer.getOffset(handle7), 0) << "This block should be starting at offset 0";
EXPECT_TRUE(checkContinuousData(buffer, handle7, { sixteen }));
}

}

0 comments on commit 908f01f

Please sign in to comment.