diff --git a/src/corelib/tools/qarraydata.cpp b/src/corelib/tools/qarraydata.cpp index cea41dbaff4..03465238b80 100644 --- a/src/corelib/tools/qarraydata.cpp +++ b/src/corelib/tools/qarraydata.cpp @@ -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; diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index f6384ca9986..8c0ca8f2f41 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -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(this->begin()), static_cast(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 Type; }; +template +struct QCommonArrayOps : QArrayOpsSelector::Type +{ + using Base = typename QArrayOpsSelector::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 struct QArrayDataOps - : QtPrivate::QArrayOpsSelector::Type + : QtPrivate::QCommonArrayOps { }; diff --git a/src/corelib/tools/qarraydatapointer.h b/src/corelib/tools/qarraydatapointer.h index d1697d64932..aebc83ba3f4 100644 --- a/src/corelib/tools/qarraydatapointer.h +++ b/src/corelib/tools/qarraydatapointer.h @@ -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 clone(QArrayData::ArrayOptions options) const { diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h index fad61bf8e61..300baeba8c0 100644 --- a/src/corelib/tools/qlist.h +++ b/src/corelib/tools/qlist.h @@ -560,10 +560,12 @@ inline void QList::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::append(QList &&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::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::emplace(qsizetype i, Args&&... args) { Q_ASSERT_X(i >= 0 && i <= d->size, "QList::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 diff --git a/tests/auto/corelib/tools/qarraydata/simplevector.h b/tests/auto/corelib/tools/qarraydata/simplevector.h index 13c859edd95..1e9812d08ed 100644 --- a/tests/auto/corelib/tools/qarraydata/simplevector.h +++ b/tests/auto/corelib/tools/qarraydata/simplevector.h @@ -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 Data; + typedef QArrayDataPointer DataPointer; public: typedef T value_type; @@ -81,6 +82,11 @@ public: { } + SimpleVector(const QArrayDataPointer &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(data), size }); + return SimpleVector(QArrayDataPointer::fromRawData(data, size)); } private: diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp index 01729270424..c27147dfdb8 100644 --- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp +++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp @@ -38,6 +38,7 @@ #include #include #include +#include // 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 i(QTypedArrayData::allocate(alloc, allocationOptions)); QArrayDataPointer s(QTypedArrayData::allocate(alloc, allocationOptions)); QArrayDataPointer o(QTypedArrayData::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; - using Data = QTypedArrayData; using DataPointer = QArrayDataPointer; 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; + Q_UNUSED(initValue); + return QArrayDataPointer(QTypedArrayData::allocate(capacity)); + }; + + const auto testRealloc = [&] (qsizetype capacity, qsizetype newSize, auto initValue) { + using Type = std::decay_t; + using DataPointer = QArrayDataPointer; + + 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; + using DataPointer = QArrayDataPointer; + + 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("initFlags"); + QTest::addColumn("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::allocateGrow(QArrayDataPointer(), 50, 0, initFlags); + auto b = QArrayDataPointer::allocateGrow(QArrayDataPointer(), 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::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 {