Support GrowsBackwards flag in QArrayDataPointer

Introduced allocation function in QArrayDataPointer with
interface similar to QArrayData::allocate that supports growing
strategies. This func is used instead of the original in cases
when prepend-aware storage is needed. Tried to follow Qt5 QList
policy in terms of space reservation

Updated QPodArrayOps::reallocate to be aware of growing
shenanigans. It doesn't look like a perfect solution but it is
rather close and similar to what Qt6 QList is doing when not
growing (e.g. reserve/squeeze)

Added initial QCommonArrayOps with helper function that tells
when reallocation is preferable over just using the insert-like
operation. This comes up later on when GrowsBackwards policy is
properly supported in operations

Essentially, 2/3 main data management blocks for prepend optimization
are introduced here. The last one being a generalized data move that
is done instead of reallocation when existing free space is not enough

Task-number: QTBUG-84320
Change-Id: I9a2bac62ad600613a6d7c5348325e0e54aadb73d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Andrei Golubev 2020-07-31 15:27:08 +02:00
parent 4b2f5371d9
commit e35d0ae0cc
6 changed files with 262 additions and 34 deletions

View File

@ -145,7 +145,7 @@ static inline qsizetype calculateBlockSize(qsizetype &capacity, qsizetype object
// Calculate the byte size
// allocSize = objectSize * capacity + headerSize, but checked for overflow
// plus padded to grow in size
if (options & QArrayData::GrowsForward) {
if (options & (QArrayData::GrowsForward | QArrayData::GrowsBackwards)) {
auto r = qCalculateGrowingBlockSize(capacity, objectSize, headerSize);
capacity = r.elementCount;
return r.size;

View File

@ -465,6 +465,19 @@ public:
void reallocate(qsizetype alloc, typename Data::ArrayOptions options)
{
// when reallocating, take care of the situation when no growth is
// happening - need to move the data in this case, unfortunately
const bool grows = options & (Data::GrowsForward | Data::GrowsBackwards);
// ### optimize me: there may be cases when moving is not obligatory
if (this->d && !grows) {
const auto gap = this->freeSpaceAtBegin();
auto oldBegin = this->begin();
this->ptr -= gap;
::memmove(static_cast<void *>(this->begin()), static_cast<void *>(oldBegin),
this->size * sizeof(T));
}
auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, options);
this->d = pair.first;
this->ptr = pair.second;
@ -1033,11 +1046,55 @@ struct QArrayOpsSelector<T,
typedef QMovableArrayOps<T> Type;
};
template <class T>
struct QCommonArrayOps : QArrayOpsSelector<T>::Type
{
using Base = typename QArrayOpsSelector<T>::Type;
using parameter_type = typename Base::parameter_type;
using iterator = typename Base::iterator;
using const_iterator = typename Base::const_iterator;
// Returns whether reallocation is desirable before adding more elements
// into the container. This is a helper function that one can use to
// theoretically improve average operations performance. Ignoring this
// function does not affect the correctness of the array operations.
bool shouldGrowBeforeInsert(const_iterator where, qsizetype n) const noexcept
{
if (this->d == nullptr)
return true;
if (this->d->flags & QArrayData::CapacityReserved)
return false;
if (!(this->d->flags & (QArrayData::GrowsForward | QArrayData::GrowsBackwards)))
return false;
Q_ASSERT(where >= this->begin() && where <= this->end()); // in range
const qsizetype freeAtBegin = this->freeSpaceAtBegin();
const qsizetype freeAtEnd = this->freeSpaceAtEnd();
const qsizetype capacity = this->constAllocatedCapacity();
if (where == this->begin()) { // prepend
// Qt5 QList in prepend: not enough space at begin && 33% full
// Now (below):
return freeAtBegin < n && (this->size >= (capacity / 3));
}
if (where == this->end()) { // append
// Qt5 QList in append: not enough space at end && less than 66% free space at front
// Now (below):
return freeAtEnd < n && !((freeAtBegin - n) >= (2 * capacity / 3));
}
// Qt5 QList in insert: no free space
// Now: no free space OR not enough space on either of the sides (bad perf. case)
return (freeAtBegin + freeAtEnd) < n || (freeAtBegin < n && freeAtEnd < n);
}
};
} // namespace QtPrivate
template <class T>
struct QArrayDataOps
: QtPrivate::QArrayOpsSelector<T>::Type
: QtPrivate::QCommonArrayOps<T>
{
};

View File

@ -209,6 +209,33 @@ public:
return d->constAllocatedCapacity() - freeSpaceAtBegin() - this->size;
}
static QArrayDataPointer allocateGrow(const QArrayDataPointer &from, qsizetype capacity,
qsizetype newSize, QArrayData::ArrayOptions options)
{
auto [header, dataPtr] = Data::allocate(capacity, options);
const bool valid = header != nullptr && dataPtr != nullptr;
const bool grows = (options & (Data::GrowsForward | Data::GrowsBackwards));
if (!valid || !grows)
return QArrayDataPointer(header, dataPtr);
// when growing, special rules apply to memory layout
if (from.needsDetach()) {
// When detaching: the free space reservation is biased towards
// append as in Qt5 QList. If we're growing backwards, put the data
// in the middle instead of at the end - assuming that prepend is
// uncommon and even initial prepend will eventually be followed by
// at least some appends.
if (options & Data::GrowsBackwards)
dataPtr += (header->alloc - newSize) / 2;
} else {
// When not detaching: fake ::realloc() policy - preserve existing
// free space at beginning.
dataPtr += from.freeSpaceAtBegin();
}
return QArrayDataPointer(header, dataPtr);
}
private:
Q_REQUIRED_RESULT QPair<Data *, T *> clone(QArrayData::ArrayOptions options) const
{

View File

@ -560,10 +560,12 @@ inline void QList<T>::append(const_iterator i1, const_iterator i2)
{
if (i1 == i2)
return;
const size_t newSize = size() + std::distance(i1, i2);
if (d->needsDetach() || newSize > d->allocatedCapacity()) {
DataPointer detached(Data::allocate(d->detachCapacity(newSize),
d->detachFlags() | Data::GrowsForward));
const size_t distance = std::distance(i1, i2);
const size_t newSize = size() + distance;
const bool shouldGrow = d->shouldGrowBeforeInsert(d.end(), qsizetype(distance));
if (d->needsDetach() || newSize > d->allocatedCapacity() || shouldGrow) {
DataPointer detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
d->detachFlags() | Data::GrowsForward));
detached->copyAppend(constBegin(), constEnd());
detached->copyAppend(i1, i2);
d.swap(detached);
@ -582,9 +584,10 @@ inline void QList<T>::append(QList<T> &&other)
return append(other);
const size_t newSize = size() + other.size();
if (d->needsDetach() || newSize > d->allocatedCapacity()) {
DataPointer detached(Data::allocate(d->detachCapacity(newSize),
d->detachFlags() | Data::GrowsForward));
const bool shouldGrow = d->shouldGrowBeforeInsert(d.end(), other.size());
if (d->needsDetach() || newSize > d->allocatedCapacity() || shouldGrow) {
DataPointer detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
d->detachFlags() | Data::GrowsForward));
if (!d->needsDetach())
detached->moveAppend(begin(), end());
@ -610,10 +613,12 @@ QList<T>::insert(qsizetype i, qsizetype n, parameter_type t)
// it's not worth wasting CPU cycles for that
const size_t newSize = size() + n;
if (d->needsDetach() || newSize > d->allocatedCapacity()) {
const bool shouldGrow = d->shouldGrowBeforeInsert(d.begin() + i, n);
if (d->needsDetach() || newSize > d->allocatedCapacity() || shouldGrow) {
typename Data::ArrayOptions flags = d->detachFlags() | Data::GrowsForward;
DataPointer detached(Data::allocate(d->detachCapacity(newSize), flags));
DataPointer detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
flags));
const_iterator where = constBegin() + i;
detached->copyAppend(constBegin(), where);
detached->copyAppend(n, t);
@ -638,11 +643,13 @@ QList<T>::emplace(qsizetype i, Args&&... args)
{
Q_ASSERT_X(i >= 0 && i <= d->size, "QList<T>::insert", "index out of range");
const bool shouldGrow = d->shouldGrowBeforeInsert(d.begin() + i, 1);
const size_t newSize = size() + 1;
if (d->needsDetach() || newSize > d->allocatedCapacity()) {
if (d->needsDetach() || newSize > d->allocatedCapacity() || shouldGrow) {
typename Data::ArrayOptions flags = d->detachFlags() | Data::GrowsForward;
DataPointer detached(Data::allocate(d->detachCapacity(newSize), flags));
DataPointer detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
flags));
const_iterator where = constBegin() + i;
// First, create an element to handle cases, when a user moves

View File

@ -1,6 +1,6 @@
/****************************************************************************
**
** Copyright (C) 2016 The Qt Company Ltd.
** Copyright (C) 2020 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/
**
** This file is part of the test suite of the Qt Toolkit.
@ -40,6 +40,7 @@ struct SimpleVector
{
private:
typedef QTypedArrayData<T> Data;
typedef QArrayDataPointer<T> DataPointer;
public:
typedef T value_type;
@ -81,6 +82,11 @@ public:
{
}
SimpleVector(const QArrayDataPointer<T> &other)
: d(other)
{
}
bool empty() const { return d.size == 0; }
bool isNull() const { return d.isNull(); }
bool isEmpty() const { return this->empty(); }
@ -195,11 +201,15 @@ public:
return;
T *const begin = d->begin();
const bool shouldGrow = d->shouldGrowBeforeInsert(d.begin(), last - first);
const auto newSize = size() + (last - first);
if (d->needsDetach()
|| capacity() - size() < size_t(last - first)) {
SimpleVector detached(Data::allocate(
d->detachCapacity(size() + (last - first)),
d->detachFlags() | Data::GrowsForward));
|| capacity() - size() < size_t(last - first)
|| shouldGrow) {
// QTBUG-84320: change to GrowsBackwards once supported in operations
auto flags = d->detachFlags() | Data::GrowsForward;
SimpleVector detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
flags));
detached.d->copyAppend(first, last);
detached.d->copyAppend(begin, begin + d->size);
@ -216,11 +226,13 @@ public:
if (first == last)
return;
const bool shouldGrow = d->shouldGrowBeforeInsert(d.end(), last - first);
const auto newSize = size() + (last - first);
if (d->needsDetach()
|| capacity() - size() < size_t(last - first)) {
SimpleVector detached(Data::allocate(
d->detachCapacity(size() + (last - first)),
d->detachFlags() | Data::GrowsForward));
|| capacity() - size() < size_t(last - first)
|| shouldGrow) {
SimpleVector detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
d->detachFlags() | Data::GrowsForward));
if (d->size) {
const T *const begin = constBegin();
@ -256,11 +268,13 @@ public:
const iterator begin = d->begin();
const iterator where = begin + position;
const iterator end = begin + d->size;
const bool shouldGrow = d->shouldGrowBeforeInsert(d.begin() + position, last - first);
const auto newSize = size() + (last - first);
if (d->needsDetach()
|| capacity() - size() < size_t(last - first)) {
SimpleVector detached(Data::allocate(
d->detachCapacity(size() + (last - first)),
d->detachFlags() | Data::GrowsForward));
|| capacity() - size() < size_t(last - first)
|| shouldGrow) {
SimpleVector detached(DataPointer::allocateGrow(d, d->detachCapacity(newSize), newSize,
d->detachFlags() | Data::GrowsForward));
if (position)
detached.d->copyAppend(begin, where);
@ -329,7 +343,7 @@ public:
static SimpleVector fromRawData(const T *data, size_t size)
{
return SimpleVector({ nullptr, const_cast<T *>(data), size });
return SimpleVector(QArrayDataPointer<T>::fromRawData(data, size));
}
private:

View File

@ -38,6 +38,7 @@
#include <algorithm>
#include <vector>
#include <stdexcept>
#include <functional>
// A wrapper for a test function. Calls a function, if it fails, reports failure
#define RUN_TEST_FUNC(test, ...) \
@ -78,6 +79,10 @@ private slots:
void grow();
void freeSpace_data();
void freeSpace();
void dataPointerAllocate_data() { arrayOps_data(); }
void dataPointerAllocate();
void dataPointerAllocateAlignedWithReallocate_data();
void dataPointerAllocateAlignedWithReallocate();
#ifndef QT_NO_EXCEPTIONS
void exceptionSafetyPrimitives_constructor();
void exceptionSafetyPrimitives_destructor();
@ -473,7 +478,8 @@ void tst_QArrayData::allocate_data()
} options[] = {
{ "Default", QArrayData::DefaultAllocationFlags, false },
{ "Reserved", QArrayData::CapacityReserved, true },
{ "Grow", QArrayData::GrowsForward, false }
{ "Grow", QArrayData::GrowsForward, false },
{ "GrowBack", QArrayData::GrowsBackwards, false }
};
for (size_t i = 0; i < sizeof(types)/sizeof(types[0]); ++i)
@ -507,7 +513,7 @@ void tst_QArrayData::allocate()
keeper.headers.append(data);
if (allocateOptions & QArrayData::GrowsForward)
if (allocateOptions & (QArrayData::GrowsForward | QArrayData::GrowsBackwards))
QVERIFY(data->allocatedCapacity() > capacity);
else
QCOMPARE(data->allocatedCapacity(), capacity);
@ -549,7 +555,7 @@ void tst_QArrayData::reallocate()
keeper.headers.clear();
keeper.headers.append(data);
if (allocateOptions & QArrayData::GrowsForward)
if (allocateOptions & (QArrayData::GrowsForward | QArrayData::GrowsBackwards))
QVERIFY(data->allocatedCapacity() > newCapacity);
else
QCOMPARE(data->allocatedCapacity(), newCapacity);
@ -1128,6 +1134,7 @@ void tst_QArrayData::arrayOpsExtra()
const auto setupDataPointers = [&allocationOptions] (size_t capacity, size_t initialSize = 0) {
const qsizetype alloc = qsizetype(capacity);
// QTBUG-84320: change to growing function once supported
QArrayDataPointer<int> i(QTypedArrayData<int>::allocate(alloc, allocationOptions));
QArrayDataPointer<QString> s(QTypedArrayData<QString>::allocate(alloc, allocationOptions));
QArrayDataPointer<CountedObject> o(QTypedArrayData<CountedObject>::allocate(alloc, allocationOptions));
@ -1839,14 +1846,14 @@ void tst_QArrayData::freeSpace()
{
QFETCH(QArrayData::ArrayOptions, allocationOptions);
QFETCH(size_t, n);
const auto testFreeSpace = [] (auto dummy, auto options, size_t n) {
const auto testFreeSpace = [] (auto dummy, auto options, qsizetype n) {
using Type = std::decay_t<decltype(dummy)>;
using Data = QTypedArrayData<Type>;
using DataPointer = QArrayDataPointer<Type>;
Q_UNUSED(dummy);
DataPointer ptr(Data::allocate(n, options));
const qsizetype capacity = n + 1;
auto ptr = DataPointer::allocateGrow(DataPointer(), capacity, n, options);
const auto alloc = qsizetype(ptr.constAllocatedCapacity());
QVERIFY(size_t(alloc) >= n);
QVERIFY(alloc >= capacity);
QCOMPARE(ptr.freeSpaceAtBegin() + ptr.freeSpaceAtEnd(), alloc);
};
RUN_TEST_FUNC(testFreeSpace, char(0), allocationOptions, n);
@ -1856,6 +1863,122 @@ void tst_QArrayData::freeSpace()
RUN_TEST_FUNC(testFreeSpace, CountedObject(), allocationOptions, n);
}
void tst_QArrayData::dataPointerAllocate()
{
QFETCH(QArrayData::ArrayOptions, allocationOptions);
const auto createDataPointer = [] (qsizetype capacity, auto initValue) {
using Type = std::decay_t<decltype(initValue)>;
Q_UNUSED(initValue);
return QArrayDataPointer<Type>(QTypedArrayData<Type>::allocate(capacity));
};
const auto testRealloc = [&] (qsizetype capacity, qsizetype newSize, auto initValue) {
using Type = std::decay_t<decltype(initValue)>;
using DataPointer = QArrayDataPointer<Type>;
auto oldDataPointer = createDataPointer(capacity, initValue);
oldDataPointer->insert(oldDataPointer.begin(), 1, initValue); // trigger prepend
QVERIFY(!oldDataPointer.needsDetach());
auto newDataPointer = DataPointer::allocateGrow(
oldDataPointer, oldDataPointer->detachCapacity(newSize), newSize, allocationOptions);
const auto newAlloc = newDataPointer.constAllocatedCapacity();
const auto freeAtBegin = newDataPointer.freeSpaceAtBegin();
const auto freeAtEnd = newDataPointer.freeSpaceAtEnd();
QVERIFY(newAlloc > oldDataPointer.constAllocatedCapacity());
QCOMPARE(size_t(freeAtBegin + freeAtEnd), newAlloc);
// when not detached, the behavior is the same as of ::realloc
if (allocationOptions & (QArrayData::GrowsForward | QArrayData::GrowsBackwards))
QCOMPARE(freeAtBegin, oldDataPointer.freeSpaceAtBegin());
else
QCOMPARE(freeAtBegin, 0);
};
for (size_t n : {10, 512, 1000}) {
RUN_TEST_FUNC(testRealloc, n, n + 1, int(0));
RUN_TEST_FUNC(testRealloc, n, n + 1, char('a'));
RUN_TEST_FUNC(testRealloc, n, n + 1, char16_t(u'a'));
RUN_TEST_FUNC(testRealloc, n, n + 1, QString("hello, world!"));
RUN_TEST_FUNC(testRealloc, n, n + 1, CountedObject());
}
const auto testDetachRealloc = [&] (qsizetype capacity, qsizetype newSize, auto initValue) {
using Type = std::decay_t<decltype(initValue)>;
using DataPointer = QArrayDataPointer<Type>;
auto oldDataPointer = createDataPointer(capacity, initValue);
oldDataPointer->insert(oldDataPointer.begin(), 1, initValue); // trigger prepend
auto oldDataPointerCopy = oldDataPointer; // force detach later
QVERIFY(oldDataPointer.needsDetach());
auto newDataPointer = DataPointer::allocateGrow(
oldDataPointer, oldDataPointer->detachCapacity(newSize), newSize, allocationOptions);
const auto newAlloc = newDataPointer.constAllocatedCapacity();
const auto freeAtBegin = newDataPointer.freeSpaceAtBegin();
const auto freeAtEnd = newDataPointer.freeSpaceAtEnd();
QVERIFY(newAlloc > oldDataPointer.constAllocatedCapacity());
QCOMPARE(size_t(freeAtBegin + freeAtEnd), newAlloc);
if (allocationOptions & QArrayData::GrowsBackwards) {
QCOMPARE(size_t(freeAtBegin), (newAlloc - newSize) / 2);
} else {
QCOMPARE(freeAtBegin, 0);
}
};
for (size_t n : {10, 512, 1000}) {
RUN_TEST_FUNC(testDetachRealloc, n, n + 1, int(0));
RUN_TEST_FUNC(testDetachRealloc, n, n + 1, char('a'));
RUN_TEST_FUNC(testDetachRealloc, n, n + 1, char16_t(u'a'));
RUN_TEST_FUNC(testDetachRealloc, n, n + 1, QString("hello, world!"));
RUN_TEST_FUNC(testDetachRealloc, n, n + 1, CountedObject());
}
}
void tst_QArrayData::dataPointerAllocateAlignedWithReallocate_data()
{
QTest::addColumn<QArrayData::ArrayOptions>("initFlags");
QTest::addColumn<QArrayData::ArrayOptions>("newFlags");
QTest::newRow("default-flags") << QArrayData::ArrayOptions(QArrayData::DefaultAllocationFlags)
<< QArrayData::ArrayOptions(QArrayData::DefaultAllocationFlags);
QTest::newRow("no-grows-backwards") << QArrayData::ArrayOptions(QArrayData::GrowsForward)
<< QArrayData::ArrayOptions(QArrayData::GrowsForward);
QTest::newRow("grows-backwards") << QArrayData::ArrayOptions(QArrayData::GrowsBackwards)
<< QArrayData::ArrayOptions(QArrayData::GrowsBackwards);
QTest::newRow("removed-grows-backwards") << QArrayData::ArrayOptions(QArrayData::GrowsBackwards)
<< QArrayData::ArrayOptions(QArrayData::GrowsForward);
QTest::newRow("removed-growth") << QArrayData::ArrayOptions(QArrayData::GrowsBackwards)
<< QArrayData::ArrayOptions(QArrayData::DefaultAllocationFlags);
}
void tst_QArrayData::dataPointerAllocateAlignedWithReallocate()
{
QFETCH(QArrayData::ArrayOptions, initFlags);
QFETCH(QArrayData::ArrayOptions, newFlags);
// Note: using the same type to ensure alignment and padding are the same.
// otherwise, we may get differences in the allocated size
auto a = QArrayDataPointer<int>::allocateGrow(QArrayDataPointer<int>(), 50, 0, initFlags);
auto b = QArrayDataPointer<int>::allocateGrow(QArrayDataPointer<int>(), 50, 0, initFlags);
if (initFlags & QArrayData::GrowsBackwards) {
QVERIFY(a.freeSpaceAtBegin() > 0);
} else {
QVERIFY(a.freeSpaceAtBegin() == 0);
}
QCOMPARE(a.freeSpaceAtBegin(), b.freeSpaceAtBegin());
a->reallocate(100, newFlags);
b = QArrayDataPointer<int>::allocateGrow(b, 100, b.size, newFlags);
// It is enough to test that the behavior of reallocate is the same as the
// behavior of allocate w.r.t. pointer adjustment in case of
// GrowsBackwards. Actual values are not that interesting
QCOMPARE(a.freeSpaceAtBegin(), b.freeSpaceAtBegin());
}
#ifndef QT_NO_EXCEPTIONS
struct ThrowingTypeWatcher
{