third_party/WebKit/Source/platform/graphics/ContiguousContainerTest.cpp - Issue 2119033003: Fix alignment issue of ContiguousContainer

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: Created 4 years, 6 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

« no previous file with comments | « third_party/WebKit/Source/platform/graphics/ContiguousContainer.cpp ('k') | third_party/WebKit/Source/platform/graphics/paint/CachedDisplayItem.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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..93e0819723df7d82d51c493c23efa503c8471d2b 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);

@@ -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), WTF_ALIGN_OF(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), WTF_ALIGN_OF(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), WTF_ALIGN_OF(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), WTF_ALIGN_OF(Point2D));

EXPECT_EQ(&movedPoint1, &list2.last());

- Point2D& movedPoint2 = list1.appendByMoving(movedPoint1, sizeof(Point2D));

+ Point2D& movedPoint2 = list1.appendByMoving(movedPoint1, sizeof(Point2D), WTF_ALIGN_OF(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), WTF_ALIGN_OF(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), WTF_ALIGN_OF(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), WTF_ALIGN_OF(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>();

@@ -524,5 +523,63 @@ TEST(ContiguousContainerTest, CapacityInBytesAfterClear)

EXPECT_EQ(emptyCapacity, list.capacityInBytes());

}

-} // namespace

+TEST(ContiguousContainerTest, Alignment)

+ ContiguousContainer<char> container(256, 256);

+ void* item1 = container.allocate(5, 128);

+ ASSERT_TRUE(item1);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item1) & 0x7F);

+ EXPECT_GE(container.usedCapacityInBytes(), 5u);

+ // The initial gap may not be consistent among runs.

+ // |initialGap|--5--|---------------------unused--------------------------|

+ // |item1|

+ size_t initialGap = container.usedCapacityInBytes() - 5;

+ void* item2 = container.allocate(11, 64);

+ ASSERT_TRUE(item2);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item2) & 0x3F);

+ // |initialGap|-------64--------|-11--|--------------unused---------------|

+ // |item1| |item2|

+ EXPECT_EQ(initialGap + 64u + 11u, container.usedCapacityInBytes());

+ void* item3 = container.allocate(13, 32);

+ ASSERT_TRUE(item3);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item3) & 0x1F);

+ // |initialGap|-------64--------|----32-----|-13--|-------unused----------|

+ EXPECT_EQ(initialGap + 64u + 32u + 13u, container.usedCapacityInBytes());

+ void* item4 = container.allocate(7, 16);

+ ASSERT_TRUE(item4);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item4) & 0x0F);

+ // |initialGap|-------64--------|----32-----|---16----|--7--|---unused----|

+ EXPECT_EQ(initialGap + 64u + 32u + 16u + 7u, container.usedCapacityInBytes());

+ // All of the above allocations should be in the initial buffer.

+ EXPECT_EQ(container.capacityInBytes(), 256u);

+ // item5 may or may not be allocated in the initial buffer, depending on

+ // initialGap.

+ void* item5 = container.allocate(9, 16);

+ ASSERT_TRUE(item5);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item5) & 0x0F);

+ // Definitely the initial buffer can't hold another small object aligned at

+ // 128 bytes, because the buffer has no more available 128-byte aligned address.

+ void* item6 = container.allocate(17, 128);

+ ASSERT_TRUE(item6);

+ EXPECT_EQ(0u, reinterpret_cast<size_t>(item6) & 0x3F);

+ EXPECT_GT(container.capacityInBytes(), 256u);

+ container.removeLast(); // item6

+ container.removeLast(); // item5

+ // The container should retain the last empty buffer to avoid reallocation of

+ // buffer for new item allocations.

+ EXPECT_GT(container.capacityInBytes(), 256u);

+ container.removeLast(); // item4

+ EXPECT_EQ(initialGap + 64u + 32u + 16u, container.usedCapacityInBytes());

} // namespace blink