Skip to content

Commit

Permalink
Resurrect data moves in QList
Browse files Browse the repository at this point in the history 
Use the data moves to readjust the free space in the QList,
which ultimately fixes the out-of-memory issues caused by
cases like:
forever {
  list.prepend(list.back());
  list.removeLast();
}

Task-number: QTBUG-91801
Task-number: QTBUG-91360
Task-number: QTBUG-93019
Pick-to: dev 6.0 6.1
Change-Id: Iacff69cbf36b8b5b176bb2663df635ec972c875c
Reviewed-by: Lars Knoll <[email protected]>
  • Loading branch information
@andrey-golubev
andrey-golubev committed Apr 26, 2021
1 parent 10b46e7 commit a0253f5
Show file tree
Hide file tree
Showing 8 changed files with 366 additions and 100 deletions.
19 changes: 5 additions & 14 deletions src/corelib/text/qbytearray.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1971,8 +1971,7 @@ QByteArray &QByteArray::append(const QByteArray &ba)

QByteArray& QByteArray::append(char ch)
{
if (d->needsDetach() || !d->freeSpaceAtEnd())
reallocGrowData(1);
d.detachAndGrow(QArrayData::GrowsAtEnd, 1, nullptr, nullptr);
d->copyAppend(1, ch);
d.data()[d.size] = '\0';
return *this;
Expand Down Expand Up @@ -2012,12 +2011,8 @@ QByteArray &QByteArray::insert(qsizetype i, QByteArrayView data)
// defer a call to free() so that it comes after we copied the data from
// the old memory:
DataPointer detached{}; // construction is free
if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) {
detached = DataPointer::allocateGrow(d, i + size - d->size, Data::GrowsAtEnd);
Q_CHECK_PTR(detached.data());
detached->copyAppend(d.constBegin(), d.constEnd());
d.swap(detached);
}
d.detachAndGrow(Data::GrowsAtEnd, (i - d.size) + size, &str, &detached);
Q_CHECK_PTR(d.data());
d->copyAppend(i - d->size, ' ');
d->copyAppend(str, str + size);
d.data()[d.size] = '\0';
Expand Down Expand Up @@ -2094,12 +2089,8 @@ QByteArray &QByteArray::insert(qsizetype i, qsizetype count, char ch)

if (i >= d->size) {
// handle this specially, as QArrayDataOps::insert() doesn't handle out of bounds positions
if (d->needsDetach() || i + count - d->size > d.freeSpaceAtEnd()) {
DataPointer detached(DataPointer::allocateGrow(d, i + count - d->size, Data::GrowsAtEnd));
Q_CHECK_PTR(detached.data());
detached->copyAppend(d.constBegin(), d.constEnd());
d.swap(detached);
}
d.detachAndGrow(Data::GrowsAtEnd, (i - d.size) + count, nullptr, nullptr);
Q_CHECK_PTR(d.data());
d->copyAppend(i - d->size, ' ');
d->copyAppend(count, ch);
d.data()[d.size] = '\0';
Expand Down
49 changes: 22 additions & 27 deletions src/corelib/text/qstring.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2770,13 +2770,17 @@ QString &QString::insert(qsizetype i, QLatin1String str)
return *this;

qsizetype len = str.size();
qsizetype difference = 0;
if (Q_UNLIKELY(i > size()))
resize(i + len, QLatin1Char(' '));
else
resize(size() + len);
difference = i - size();
d.detachAndGrow(Data::GrowsAtEnd, difference + len, nullptr, nullptr);
Q_CHECK_PTR(d.data());
d->copyAppend(difference, u' ');
d.size += len;

::memmove(d.data() + i + len, d.data() + i, (d.size - i - len) * sizeof(QChar));
qt_from_latin1(d.data() + i, s, size_t(len));
d.data()[d.size] = u'\0';
return *this;
}

Expand All @@ -2797,7 +2801,7 @@ QString& QString::insert(qsizetype i, const QChar *unicode, qsizetype size)
if (i < 0 || size <= 0)
return *this;

const auto s = reinterpret_cast<const char16_t *>(unicode);
const char16_t *s = reinterpret_cast<const char16_t *>(unicode);

// handle this specially, as QArrayDataOps::insert() doesn't handle out of
// bounds positions
Expand All @@ -2806,12 +2810,8 @@ QString& QString::insert(qsizetype i, const QChar *unicode, qsizetype size)
// defer a call to free() so that it comes after we copied the data from
// the old memory:
DataPointer detached{}; // construction is free
if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) {
detached = DataPointer::allocateGrow(d, i + size - d->size, Data::GrowsAtEnd);
Q_CHECK_PTR(detached.data());
detached->copyAppend(d.constBegin(), d.constEnd());
d.swap(detached);
}
d.detachAndGrow(Data::GrowsAtEnd, (i - d.size) + size, &s, &detached);
Q_CHECK_PTR(d.data());
d->copyAppend(i - d->size, u' ');
d->copyAppend(s, s + size);
d.data()[d.size] = u'\0';
Expand Down Expand Up @@ -2867,11 +2867,8 @@ QString &QString::append(const QString &str)
if (!str.isNull()) {
if (isNull()) {
operator=(str);
} else {
if (d->needsDetach() || str.size() > d->freeSpaceAtEnd())
reallocGrowData(str.size());
d->copyAppend(str.d.data(), str.d.data() + str.d.size);
d.data()[d.size] = '\0';
} else if (str.size()) {
append(str.constData(), str.size());
}
}
return *this;
Expand All @@ -2886,13 +2883,11 @@ QString &QString::append(const QString &str)
QString &QString::append(const QChar *str, qsizetype len)
{
if (str && len > 0) {
if (d->needsDetach() || len > d->freeSpaceAtEnd())
reallocGrowData(len);
static_assert(sizeof(QChar) == sizeof(char16_t), "Unexpected difference in sizes");
// the following should be safe as QChar uses char16_t as underlying data
const char16_t *char16String = reinterpret_cast<const char16_t *>(str);
d->copyAppend(char16String, char16String + len);
d.data()[d.size] = '\0';
d->growAppend(char16String, char16String + len);
d.data()[d.size] = u'\0';
}
return *this;
}
Expand All @@ -2905,16 +2900,17 @@ QString &QString::append(const QChar *str, qsizetype len)
QString &QString::append(QLatin1String str)
{
const char *s = str.latin1();
if (s) {
qsizetype len = str.size();
if (d->needsDetach() || str.size() > d->freeSpaceAtEnd())
reallocGrowData(len);

Q_ASSERT(str.size() <= d->freeSpaceAtEnd());
const qsizetype len = str.size();
if (s && len > 0) {
d.detachAndGrow(Data::GrowsAtEnd, len, nullptr, nullptr);
Q_CHECK_PTR(d.data());
Q_ASSERT(len <= d->freeSpaceAtEnd());
char16_t *i = d.data() + d.size;
qt_from_latin1(i, s, size_t(len));
d.size += len;
d.data()[d.size] = '\0';
} else if (d.isNull() && !str.isNull()) { // special case
d = DataPointer::fromRawData(&_empty, 0);
}
return *this;
}
Expand Down Expand Up @@ -2952,8 +2948,7 @@ QString &QString::append(QLatin1String str)
*/
QString &QString::append(QChar ch)
{
if (d->needsDetach() || !d->freeSpaceAtEnd())
reallocGrowData(1);
d.detachAndGrow(QArrayData::GrowsAtEnd, 1, nullptr, nullptr);
d->copyAppend(1, ch.unicode());
d.data()[d.size] = '\0';
return *this;
Expand Down
56 changes: 38 additions & 18 deletions src/corelib/tools/qarraydataops.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,9 @@ struct QPodArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;

DataPointer oldData;
this->detachAndGrow(pos, n, &oldData);
this->detachAndGrow(pos, n, &data, &oldData);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand All @@ -180,7 +181,8 @@ struct QPodArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;
this->detachAndGrow(pos, n);

this->detachAndGrow(pos, n, nullptr, nullptr);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand Down Expand Up @@ -210,10 +212,8 @@ struct QPodArrayOps
typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = QArrayData::GrowsAtBeginning;
if (detach ||
(pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
(pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
this->reallocateAndGrow(pos, 1);

this->detachAndGrow(pos, 1, nullptr, nullptr);

T *where = createHole(pos, i, 1);
new (where) T(std::move(tmp));
Expand Down Expand Up @@ -547,8 +547,9 @@ struct QGenericArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;

DataPointer oldData;
this->detachAndGrow(pos, n, &oldData);
this->detachAndGrow(pos, n, &data, &oldData);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand All @@ -562,7 +563,8 @@ struct QGenericArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;
this->detachAndGrow(pos, n);

this->detachAndGrow(pos, n, nullptr, nullptr);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand Down Expand Up @@ -590,10 +592,8 @@ struct QGenericArrayOps
typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = QArrayData::GrowsAtBeginning;
if (detach ||
(pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
(pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
this->reallocateAndGrow(pos, 1);

this->detachAndGrow(pos, 1, nullptr, nullptr);

Inserter(this, pos).insertOne(i, std::move(tmp));
}
Expand Down Expand Up @@ -773,8 +773,9 @@ struct QMovableArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;

DataPointer oldData;
this->detachAndGrow(pos, n, &oldData);
this->detachAndGrow(pos, n, &data, &oldData);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand All @@ -788,7 +789,8 @@ struct QMovableArrayOps
typename Data::GrowthPosition pos = Data::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = Data::GrowsAtBeginning;
this->detachAndGrow(pos, n);

this->detachAndGrow(pos, n, nullptr, nullptr);
Q_ASSERT((pos == Data::GrowsAtBeginning && this->freeSpaceAtBegin() >= n) ||
(pos == Data::GrowsAtEnd && this->freeSpaceAtEnd() >= n));

Expand Down Expand Up @@ -816,10 +818,8 @@ struct QMovableArrayOps
typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
if (this->size != 0 && i <= (this->size >> 1))
pos = QArrayData::GrowsAtBeginning;
if (detach ||
(pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
(pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
this->reallocateAndGrow(pos, 1);

this->detachAndGrow(pos, 1, nullptr, nullptr);

Inserter(this, pos).insertOne(i, std::move(tmp));
}
Expand Down Expand Up @@ -913,6 +913,26 @@ struct QCommonArrayOps : QArrayOpsSelector<T>::Type
++this->size;
}
}

// slightly higher level API than copyAppend() that also preallocates space
void growAppend(const T *b, const T *e)
{
if (b == e)
return;
Q_ASSERT(b < e);
const qsizetype n = e - b;
DataPointer old;

// points into range:
if (QtPrivate::q_points_into_range(b, this->begin(), this->end())) {
this->detachAndGrow(QArrayData::GrowsAtEnd, n, &b, &old);
} else {
this->detachAndGrow(QArrayData::GrowsAtEnd, n, nullptr, nullptr);
}
Q_ASSERT(this->freeSpaceAtEnd() >= n);
// b might be updated so use [b, n)
this->copyAppend(b, b + n);
}
};

} // namespace QtPrivate
Expand Down
Loading

0 comments on commit a0253f5

Please sign in to comment.