Don't move data in QArrayDataOps::reallocate()

reallocate() should only ever call realloc(), and only be used to
create more space at the end of the data.

Change-Id: I2ac4dbc90d2afaa571bb620108d7984356712cb2
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
This commit is contained in:
Lars Knoll 2020-10-30 13:42:09 +01:00
parent 50ec3252ac
commit 32a703e277
4 changed files with 7 additions and 68 deletions

View File

@ -671,7 +671,7 @@ QByteArray qUncompress(const uchar* data, qsizetype nbytes)
return invalidCompressedData();
} else {
// grow the block
d->reallocate(d->allocatedCapacity()*2, QByteArray::Data::GrowsForward);
d->reallocate(d->allocatedCapacity()*2);
if (Q_UNLIKELY(d.data() == nullptr))
return invalidCompressedData();
}
@ -1719,7 +1719,7 @@ void QByteArray::reallocData(qsizetype alloc, Data::ArrayOptions options)
dd.data()[dd.size] = 0;
d = dd;
} else {
d->reallocate(alloc, options);
d->reallocate(alloc);
}
}
@ -1734,7 +1734,7 @@ void QByteArray::reallocGrowData(qsizetype n)
dd.data()[dd.size] = 0;
d = dd;
} else {
d->reallocate(d.constAllocatedCapacity() + n, Data::GrowsForward);
d->reallocate(d.constAllocatedCapacity() + n);
}
}

View File

@ -2516,7 +2516,7 @@ void QString::reallocData(qsizetype alloc, Data::ArrayOptions allocOptions)
dd.data()[dd.size] = 0;
d = dd;
} else {
d->reallocate(alloc, allocOptions);
d->reallocate(alloc);
}
}
@ -2531,7 +2531,7 @@ void QString::reallocGrowData(qsizetype n)
dd.data()[dd.size] = 0;
d = dd;
} else {
d->reallocate(d.constAllocatedCapacity() + n, Data::GrowsForward);
d->reallocate(d.constAllocatedCapacity() + n);
}
}

View File

@ -457,21 +457,9 @@ public:
}
}
void reallocate(qsizetype alloc, typename Data::ArrayOptions options)
void reallocate(qsizetype alloc)
{
// 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 (const auto gap = this->freeSpaceAtBegin(); this->d && !grows && gap) {
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);
auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, QArrayData::GrowsBackwards);
this->d = pair.first;
this->ptr = pair.second;
}

View File

@ -82,8 +82,6 @@ private slots:
void freeSpace();
void dataPointerAllocate_data() { arrayOps_data(); }
void dataPointerAllocate();
void dataPointerAllocateAlignedWithReallocate_data();
void dataPointerAllocateAlignedWithReallocate();
#ifndef QT_NO_EXCEPTIONS
void exceptionSafetyPrimitives_constructor();
void exceptionSafetyPrimitives_destructor();
@ -2158,53 +2156,6 @@ void tst_QArrayData::dataPointerAllocate()
}
}
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());
const auto oldSpaceAtBeginA = a.freeSpaceAtBegin();
a->reallocate(100, newFlags);
b = QArrayDataPointer<int>::allocateGrow(b, 100, b.size, newFlags);
// NB: when growing backwards, the behavior is not aligned
if (!(newFlags & QArrayData::GrowsBackwards)) {
QCOMPARE(a.freeSpaceAtBegin(), b.freeSpaceAtBegin());
} else {
QCOMPARE(a.freeSpaceAtBegin(), oldSpaceAtBeginA);
QCOMPARE(b.freeSpaceAtBegin(), b.constAllocatedCapacity() / 2);
}
}
#ifndef QT_NO_EXCEPTIONS
struct ThrowingTypeWatcher
{