Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(454)

Unified Diff: third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp

Issue 2119033003: Fix alignment issue of ContiguousContainer (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp b/third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp
index 656218bec359e6cf451826de31b5b1dcf669c5f3..3725135107d908e86b261163917e072746cb6f61 100644
--- a/third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp
@@ -26,15 +26,14 @@ struct Point3D : public Point2D {
// Maximum size of a subclass of Point2D.
static const size_t kMaxPointSize = sizeof(Point3D);
-// Alignment for Point2D and its subclasses.
-static const size_t kPointAlignment = sizeof(int);
-
// How many elements to use for tests with "plenty" of elements.
static const unsigned kNumElements = 150;
+} // namespace
+
TEST(ContiguousContainerTest, SimpleStructs)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
list.allocateAndConstruct<Point2D>(1, 2);
list.allocateAndConstruct<Point3D>(3, 4, 5);
list.allocateAndConstruct<Point2D>(6, 7);
@@ -51,7 +50,7 @@ TEST(ContiguousContainerTest, SimpleStructs)
TEST(ContiguousContainerTest, AllocateLots)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
for (int i = 0; i < (int)kNumElements; i++) {
list.allocateAndConstruct<Point2D>(i, i);
list.allocateAndConstruct<Point2D>(i, i);
@@ -173,7 +172,7 @@ TEST(ContiguousContainerTest, DestructorCalledWithMultipleRemoveLastCalls)
TEST(ContiguousContainerTest, InsertionAndIndexedAccess)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
auto& point1 = list.allocateAndConstruct<Point2D>();
auto& point2 = list.allocateAndConstruct<Point2D>();
@@ -189,7 +188,7 @@ TEST(ContiguousContainerTest, InsertionAndIndexedAccess)
TEST(ContiguousContainerTest, InsertionAndClear)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
EXPECT_TRUE(list.isEmpty());
EXPECT_EQ(0u, list.size());
@@ -208,7 +207,7 @@ TEST(ContiguousContainerTest, InsertionAndClear)
TEST(ContiguousContainerTest, ElementAddressesAreStable)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
Vector<Point2D*> pointers;
for (int i = 0; i < (int)kNumElements; i++)
pointers.append(&list.allocateAndConstruct<Point2D>());
@@ -223,7 +222,7 @@ TEST(ContiguousContainerTest, ElementAddressesAreStable)
TEST(ContiguousContainerTest, ForwardIteration)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
for (int i = 0; i < (int)kNumElements; i++)
list.allocateAndConstruct<Point2D>(i, i);
unsigned count = 0;
@@ -239,7 +238,7 @@ TEST(ContiguousContainerTest, ForwardIteration)
TEST(ContiguousContainerTest, ConstForwardIteration)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
for (int i = 0; i < (int)kNumElements; i++)
list.allocateAndConstruct<Point2D>(i, i);
@@ -257,7 +256,7 @@ TEST(ContiguousContainerTest, ConstForwardIteration)
TEST(ContiguousContainerTest, ReverseIteration)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
for (int i = 0; i < (int)kNumElements; i++)
list.allocateAndConstruct<Point2D>(i, i);
@@ -341,12 +340,12 @@ TEST(ContiguousContainerTest, IterationAfterRemoveLast)
TEST(ContiguousContainerTest, AppendByMovingSameList)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
list.allocateAndConstruct<Point3D>(1, 2, 3);
// Moves the Point3D to the end, and default-constructs a Point2D in its
// place.
- list.appendByMoving(list.first(), sizeof(Point3D));
+ list.appendByMoving(list.first(), sizeof(Point3D), log2Alignment<Point3D>());
EXPECT_EQ(1, list.last().x);
EXPECT_EQ(2, list.last().y);
EXPECT_EQ(3, static_cast<const Point3D&>(list.last()).z);
@@ -355,7 +354,7 @@ TEST(ContiguousContainerTest, AppendByMovingSameList)
// Moves that Point2D to the end, and default-constructs another in its
// place.
list.first().x = 4;
- list.appendByMoving(list.first(), sizeof(Point2D));
+ list.appendByMoving(list.first(), sizeof(Point2D), log2Alignment<Point2D>());
EXPECT_EQ(4, list.last().x);
EXPECT_EQ(3u, list.size());
}
@@ -378,7 +377,7 @@ TEST(ContiguousContainerTest, AppendByMovingDoesNotDestruct)
{
// Make sure destructor isn't called during appendByMoving.
ContiguousContainer<DestructionNotifier> list2(sizeof(DestructionNotifier));
- list2.appendByMoving(list1.last(), sizeof(DestructionNotifier));
+ list2.appendByMoving(list1.last(), sizeof(DestructionNotifier), log2Alignment<DestructionNotifier>());
EXPECT_FALSE(destroyed);
}
// But it should be destroyed when list2 is.
@@ -387,28 +386,28 @@ TEST(ContiguousContainerTest, AppendByMovingDoesNotDestruct)
TEST(ContiguousContainerTest, AppendByMovingReturnsMovedPointer)
{
- ContiguousContainer<Point2D, kPointAlignment> list1(kMaxPointSize);
- ContiguousContainer<Point2D, kPointAlignment> list2(kMaxPointSize);
+ ContiguousContainer<Point2D> list1(kMaxPointSize);
+ ContiguousContainer<Point2D> list2(kMaxPointSize);
Point2D& point = list1.allocateAndConstruct<Point2D>();
- Point2D& movedPoint1 = list2.appendByMoving(point, sizeof(Point2D));
+ Point2D& movedPoint1 = list2.appendByMoving(point, sizeof(Point2D), log2Alignment<Point2D>());
EXPECT_EQ(&movedPoint1, &list2.last());
- Point2D& movedPoint2 = list1.appendByMoving(movedPoint1, sizeof(Point2D));
+ Point2D& movedPoint2 = list1.appendByMoving(movedPoint1, sizeof(Point2D), log2Alignment<Point2D>());
EXPECT_EQ(&movedPoint2, &list1.last());
EXPECT_NE(&movedPoint1, &movedPoint2);
}
TEST(ContiguousContainerTest, AppendByMovingReplacesSourceWithNewElement)
{
- ContiguousContainer<Point2D, kPointAlignment> list1(kMaxPointSize);
- ContiguousContainer<Point2D, kPointAlignment> list2(kMaxPointSize);
+ ContiguousContainer<Point2D> list1(kMaxPointSize);
+ ContiguousContainer<Point2D> list2(kMaxPointSize);
list1.allocateAndConstruct<Point2D>(1, 2);
EXPECT_EQ(1, list1.first().x);
EXPECT_EQ(2, list1.first().y);
- list2.appendByMoving(list1.first(), sizeof(Point2D));
+ list2.appendByMoving(list1.first(), sizeof(Point2D), log2Alignment<Point2D>());
EXPECT_EQ(0, list1.first().x);
EXPECT_EQ(0, list1.first().y);
EXPECT_EQ(1, list2.first().x);
@@ -420,7 +419,7 @@ TEST(ContiguousContainerTest, AppendByMovingReplacesSourceWithNewElement)
TEST(ContiguousContainerTest, AppendByMovingElementsOfDifferentSizes)
{
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
list.allocateAndConstruct<Point3D>(1, 2, 3);
list.allocateAndConstruct<Point2D>(4, 5);
@@ -432,14 +431,14 @@ TEST(ContiguousContainerTest, AppendByMovingElementsOfDifferentSizes)
// Test that moving the first element actually moves the entire object, not
// just the base element.
- list.appendByMoving(list[0], sizeof(Point3D));
+ list.appendByMoving(list[0], sizeof(Point3D), log2Alignment<Point3D>());
EXPECT_EQ(1, list[2].x);
EXPECT_EQ(2, list[2].y);
EXPECT_EQ(3, static_cast<const Point3D&>(list[2]).z);
EXPECT_EQ(4, list[1].x);
EXPECT_EQ(5, list[1].y);
- list.appendByMoving(list[1], sizeof(Point2D));
+ list.appendByMoving(list[1], sizeof(Point2D), log2Alignment<Point2D>());
EXPECT_EQ(1, list[2].x);
EXPECT_EQ(2, list[2].y);
EXPECT_EQ(3, static_cast<const Point3D&>(list[2]).z);
@@ -449,9 +448,9 @@ TEST(ContiguousContainerTest, AppendByMovingElementsOfDifferentSizes)
TEST(ContiguousContainerTest, Swap)
{
- ContiguousContainer<Point2D, kPointAlignment> list1(kMaxPointSize);
+ ContiguousContainer<Point2D> list1(kMaxPointSize);
list1.allocateAndConstruct<Point2D>(1, 2);
- ContiguousContainer<Point2D, kPointAlignment> list2(kMaxPointSize);
+ ContiguousContainer<Point2D> list2(kMaxPointSize);
list2.allocateAndConstruct<Point2D>(3, 4);
list2.allocateAndConstruct<Point2D>(5, 6);
@@ -491,7 +490,7 @@ TEST(ContiguousContainerTest, CapacityInBytes)
// need to be adjusted.
const size_t maxWasteFactor = 8;
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize, initialCapacity);
+ ContiguousContainer<Point2D> list(kMaxPointSize, initialCapacity);
// The capacity should grow with the list.
for (int i = 0; i < iterations; i++) {
@@ -516,7 +515,7 @@ TEST(ContiguousContainerTest, CapacityInBytesAfterClear)
{
// Clearing should restore the capacity of the container to the same as a
// newly allocated one (without reserved capacity requested).
- ContiguousContainer<Point2D, kPointAlignment> list(kMaxPointSize);
+ ContiguousContainer<Point2D> list(kMaxPointSize);
size_t emptyCapacity = list.capacityInBytes();
list.allocateAndConstruct<Point2D>();
list.allocateAndConstruct<Point2D>();
@@ -524,5 +523,59 @@ TEST(ContiguousContainerTest, CapacityInBytesAfterClear)
EXPECT_EQ(emptyCapacity, list.capacityInBytes());
}
-} // namespace
+TEST(ContiguousContainerTest, Alignment)
+{
+ ContiguousContainer<char> container(64, 64);
+ void* item1 = container.allocate(21, 4); // alignment 16
+ ASSERT_TRUE(item1);
+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item1) & 0x0F);
+ EXPECT_GE(21u, container.usedCapacityInBytes());
+ // The initial gap may not be consistent among runs.
+ // |--------21-------|-------------------43-----------------------|
+ // | item1 | unused |
+
+ void* item2 = container.allocate(9, 3); // alignment 8
+ ASSERT_TRUE(item2);
+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item2) & 7);
+ // |---------24--------|---9---|----------------31----------------|
+ // | item1 | | item2 | unused |
+ EXPECT_EQ(24u + 9u, container.usedCapacityInBytes());
+
+ void* item3 = container.allocate(3, 2); // alignment 4
+ ASSERT_TRUE(item3);
+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item3) & 3);
+ // |---------24--------|----12----|--3--|----------25-------------|
+ // | item1 | | item2 | |item3| unused |
+ EXPECT_EQ(24u + 12u + 3u, container.usedCapacityInBytes());
+
+ void* item4 = container.allocate(11, 0);
+ ASSERT_TRUE(item4);
+ // |---------24--------|----12----|--3--|---11---|-------14-------|
+ // | item1 | | item2 | |item3| item4 | unused |
+ EXPECT_EQ(24u + 12u + 3u + 11u, container.usedCapacityInBytes());
+
+ // All of the above allocations should be in the initial buffer.
+ EXPECT_EQ(64u, container.capacityInBytes());
+
+ // The initial buffer can't hold another object aligned at 16 bytes.
+ void* item5 = container.allocate(8, 4);
+ ASSERT_TRUE(item5);
+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item5) & 0x0F);
+ EXPECT_GT(container.capacityInBytes(), 64u);
+
+ container.removeLast(); // item5
+ // The container should retain the last empty buffer to avoid reallocation of
+ // buffer for new item allocations.
+ EXPECT_GT(container.capacityInBytes(), 64u);
+
+ container.removeLast(); // item4
+ EXPECT_EQ(24u + 12u + 3u, container.usedCapacityInBytes());
+ container.removeLast(); // item3
+ EXPECT_EQ(24u + 12u, container.usedCapacityInBytes());
+ container.removeLast(); // item2
+ EXPECT_EQ(24u, container.usedCapacityInBytes());
+ container.removeLast(); // item1
+ EXPECT_EQ(0u, container.usedCapacityInBytes());
+}
+
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698