Skip to content

Commit

Permalink
Don't grow container when desired size is known
Browse files Browse the repository at this point in the history
QList<Type>::reserve() is used upfront to allocate necessary memory in a one
go. This tells us straight away whether allocation is possible at all and
reduces re-allocations and consequent memory copies.

This also has the side effect that no spare memory is allocated, also allowing
up to (and including) INT_MAX elements to actually be stored in the underlying
QList, as long as enough memory is available to satisfy the allocation request
and subsequent fill.

The qqmlecmascript::sequenceConversionIndexes was changed to not attempt
INT_MAX allocations as, given enough memory and virtual address space, that
might succeed but take a really long time.

Change-Id: I4b0c965e9c23be78874343a70d7c155933c80903
Reviewed-by: Chris Adams <christopher.adams@nokia.com>
Reviewed-by: Kent Hansen <kent.hansen@nokia.com>
  • Loading branch information
João Abecasis authored and Qt by Nokia committed Mar 9, 2012
1 parent 8fd3c01 commit 3c94c7e
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 29 deletions.
25 changes: 14 additions & 11 deletions src/qml/qml/v8/qv8sequencewrapper_p_p.h
Expand Up @@ -265,6 +265,7 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v)
static QVariant toVariant(QV8Engine *e, v8::Handle<v8::Array> array, uint32_t length, bool *succeeded) \
{ \
SequenceType list; \
list.reserve(length); \
for (uint32_t ii = 0; ii < length; ++ii) { \
list.append(ConversionFromV8fn(e, array->Get(ii))); \
} \
Expand Down Expand Up @@ -334,14 +335,15 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v)
/* according to ECMA262r3 we need to insert */ \
/* undefined values increasing length to newLength. */ \
/* We cannot, so we insert default-values instead. */ \
QT_TRY { \
c.reserve(newCount); \
} QT_CATCH (std::bad_alloc &exception) { \
generateWarning(engine, QString(QLatin1String(exception.what()) \
+ QLatin1String(" during length set"))); \
return; /* failed; don't write back any result. */ \
} \
while (newCount > count++) { \
QT_TRY { \
c.append(DefaultValue); \
} QT_CATCH (std::bad_alloc &exception) { \
generateWarning(engine, QString(QLatin1String(exception.what()) \
+ QLatin1String(" during length set"))); \
return; /* failed; don't write back any result. */ \
} \
c.append(DefaultValue); \
} \
} else { \
/* according to ECMA262r3 we need to remove */ \
Expand Down Expand Up @@ -382,15 +384,16 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v)
/* according to ECMA262r3 we need to insert */ \
/* the value at the given index, increasing length to index+1. */ \
QT_TRY { \
while (signedIdx > count++) { \
c.append(DefaultValue); \
} \
c.append(elementValue); \
c.reserve(signedIdx + 1); \
} QT_CATCH (std::bad_alloc &exception) { \
generateWarning(engine, QString(QLatin1String(exception.what()) \
+ QLatin1String(" during indexed set"))); \
return v8::Undefined(); /* failed; don't write back any result. */ \
} \
while (signedIdx > count++) { \
c.append(DefaultValue); \
} \
c.append(elementValue); \
} \
/* write back. already checked that object is non-null, so skip that check here. */ \
if (objectType == QV8SequenceResource::Reference) \
Expand Down
14 changes: 0 additions & 14 deletions tests/auto/qml/qqmlecmascript/data/sequenceConversion.indexes.qml
Expand Up @@ -71,19 +71,5 @@ Item {
success = false;
if (!verifyExpected(msco.intListProperty, 4))
success = false;

// NOTE: while these two operations are technically
// fine, we expect std::bad_alloc exceptions here
// which we handle in the sequence wrapper.
msco.intListProperty.length = maxIndex;
if (msco.intListProperty.length != expectedLength)
success = false;
if (!verifyExpected(msco.intListProperty, 4))
success = false;
msco.intListProperty[maxIndex] = 15;
if (msco.intListProperty.length != expectedLength)
success = false;
if (!verifyExpected(msco.intListProperty, 4))
success = false;
}
}
4 changes: 0 additions & 4 deletions tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
Expand Up @@ -4649,13 +4649,9 @@ void tst_qqmlecmascript::sequenceConversionIndexes()
QString w1 = qmlFile.toString() + QLatin1String(":34: Index out of range during length set");
QString w2 = qmlFile.toString() + QLatin1String(":41: Index out of range during indexed set");
QString w3 = qmlFile.toString() + QLatin1String(":48: Index out of range during indexed get");
QString w4 = qmlFile.toString() + QLatin1String(":78: std::bad_alloc during length set");
QString w5 = qmlFile.toString() + QLatin1String(":83: std::bad_alloc during indexed set");
QTest::ignoreMessage(QtWarningMsg, qPrintable(w1));
QTest::ignoreMessage(QtWarningMsg, qPrintable(w2));
QTest::ignoreMessage(QtWarningMsg, qPrintable(w3));
QTest::ignoreMessage(QtWarningMsg, qPrintable(w4));
QTest::ignoreMessage(QtWarningMsg, qPrintable(w5));
QMetaObject::invokeMethod(object, "indexedAccess");
QVERIFY(object->property("success").toBool());
delete object;
Expand Down

0 comments on commit 3c94c7e

Please sign in to comment.