Improve performance of QScriptValueIterator

The implementation was getting all the property names
(identifiers) and storing their string representation.
In e.g. value() and setValue(), the string representation
then had to be converted back to an identifier. This was
unnecessary work.

Instead, store the identifiers directly, and use them
via the QScriptValue private API whenever we can.
This greatly improves the performance of scriptName(), value(),
setValue(), flags() and remove().
(tests/benchmarks/script/qscriptvalueiterator is 4-5 times faster.)

Reviewed-by: Jedrzej Nowacki

1 files changed, 34 insertions, 21 deletions

diff --git a/src/script/api/qscriptvalueiterator.cpp b/src/script/api/qscriptvalueiterator.cpp
index 5737435..5c1e6f2 100644
--- a/src/script/api/qscriptvalueiterator.cpp
+++ b/src/script/api/qscriptvalueiterator.cpp
@@ -84,27 +84,38 @@ public:
     QScriptValueIteratorPrivate()
         : initialized(false)
     {}
+
+    QScriptValuePrivate *object() const
+    {
+        return QScriptValuePrivate::get(objectValue);
+    }
+
+    QScriptEnginePrivate *engine() const
+    {
+        return QScriptEnginePrivate::get(objectValue.engine());
+    }
+
     void ensureInitialized()
     {
         if (initialized)
             return;
-        QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(object.engine());
+        QScriptEnginePrivate *eng_p = engine();
         JSC::ExecState *exec = eng_p->globalExec();
         JSC::PropertyNameArray propertyNamesArray(exec);
-        JSC::asObject(QScriptValuePrivate::get(object)->jscValue)->getOwnPropertyNames(exec, propertyNamesArray, JSC::IncludeDontEnumProperties);
+        JSC::asObject(object()->jscValue)->getOwnPropertyNames(exec, propertyNamesArray, JSC::IncludeDontEnumProperties);
 
         JSC::PropertyNameArray::const_iterator propertyNamesIt = propertyNamesArray.begin();
         for(; propertyNamesIt != propertyNamesArray.end(); ++propertyNamesIt) {
-            propertyNames.append(propertyNamesIt->ustring());
+            propertyNames.append(*propertyNamesIt);
         }
         it = propertyNames.begin();
         initialized = true;
     }
 
-    QScriptValue object;
-    QLinkedList<JSC::UString> propertyNames;
-    QLinkedList<JSC::UString>::iterator it;
-    QLinkedList<JSC::UString>::iterator current;
+    QScriptValue objectValue;
+    QLinkedList<JSC::Identifier> propertyNames;
+    QLinkedList<JSC::Identifier>::iterator it;
+    QLinkedList<JSC::Identifier>::iterator current;
     bool initialized;
 };
 
@@ -118,7 +129,7 @@ QScriptValueIterator::QScriptValueIterator(const QScriptValue &object)
 {
     if (object.isObject()) {
         d_ptr.reset(new QScriptValueIteratorPrivate());
-        d_ptr->object = object;
+        d_ptr->objectValue = object;
     }
 }
 
@@ -239,9 +250,9 @@ void QScriptValueIterator::toBack()
 QString QScriptValueIterator::name() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return QString();
-    return *d->current;
+    return d->current->ustring();
 }
 
 /*!
@@ -253,9 +264,9 @@ QString QScriptValueIterator::name() const
 QScriptString QScriptValueIterator::scriptName() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return QScriptString();
-    return d->object.engine()->toStringHandle(name());
+    return d->engine()->toStringHandle(*d->current);
 }
 
 /*!
@@ -267,9 +278,10 @@ QScriptString QScriptValueIterator::scriptName() const
 QScriptValue QScriptValueIterator::value() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return QScriptValue();
-    return d->object.property(name());
+    JSC::JSValue jsValue = d->object()->property(*d->current);
+    return d->engine()->scriptValueFromJSCValue(jsValue);
 }
 
 /*!
@@ -281,9 +293,10 @@ QScriptValue QScriptValueIterator::value() const
 void QScriptValueIterator::setValue(const QScriptValue &value)
 {
     Q_D(QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return;
-    d->object.setProperty(name(), value);
+    JSC::JSValue jsValue = d->engine()->scriptValueToJSCValue(value);
+    d->object()->setProperty(*d->current, jsValue);
 }
 
 /*!
@@ -295,9 +308,9 @@ void QScriptValueIterator::setValue(const QScriptValue &value)
 QScriptValue::PropertyFlags QScriptValueIterator::flags() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return 0;
-    return d->object.propertyFlags(name());
+    return d->object()->propertyFlags(*d->current);
 }
 
 /*!
@@ -309,9 +322,9 @@ QScriptValue::PropertyFlags QScriptValueIterator::flags() const
 void QScriptValueIterator::remove()
 {
     Q_D(QScriptValueIterator);
-    if (!d || !d->initialized)
+    if (!d || !d->initialized || !d->engine())
         return;
-    d->object.setProperty(name(), QScriptValue());
+    d->object()->setProperty(*d->current, JSC::JSValue());
     d->propertyNames.erase(d->current);
 }
 
@@ -325,7 +338,7 @@ QScriptValueIterator& QScriptValueIterator::operator=(QScriptValue &object)
     d_ptr.reset();
     if (object.isObject()) {
         d_ptr.reset(new QScriptValueIteratorPrivate());
-        d_ptr->object = object;
+        d_ptr->objectValue = object;
     }
     return *this;
 }