Avoid creating array attributes if possible

Holes in arrays should be represented by an empty
value, not by creating/setting array attributes.

Reason is that the creation is irreversable, and slows
down execution. This speeds up crypto.js by 10%

Change-Id: I2e5472575479a5f2dbe53f59ecb8ed3aeab1be7a
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

6 files changed, 47 insertions, 57 deletions

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index c815036550..47b149c9d5 100644
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -127,12 +127,11 @@ bool ArgumentsObject::defineOwnProperty(ExecutionContext *ctx, uint index, const
     isNonStrictArgumentsObject = true;
 
     if (isMapped && attrs.isData()) {
-        if (!attrs.isGeneric()) {
-            ScopedCallData callData(scope, 1);
-            callData->thisObject = this->asReturnedValue();
-            callData->args[0] = desc.value;
-            map.setter()->call(callData);
-        }
+        ScopedCallData callData(scope, 1);
+        callData->thisObject = this->asReturnedValue();
+        callData->args[0] = desc.value;
+        map.setter()->call(callData);
+
         if (attrs.isWritable()) {
             *pd = map;
             arrayAttributes[pidx] = mapAttrs;
diff --git a/src/qml/jsruntime/qv4arrayobject.cpp b/src/qml/jsruntime/qv4arrayobject.cpp
index 92fa196331..3913b9ffe7 100644
--- a/src/qml/jsruntime/qv4arrayobject.cpp
+++ b/src/qml/jsruntime/qv4arrayobject.cpp
@@ -350,7 +350,7 @@ ReturnedValue ArrayPrototype::method_shift(SimpleCallContext *ctx)
 
     Property *front = 0;
     uint pidx = instance->propertyIndexFromArrayIndex(0);
-    if (pidx < UINT_MAX && (!instance->arrayAttributes || !instance->arrayAttributes[0].isGeneric()))
+    if (pidx < UINT_MAX && !instance->arrayData[pidx].value.isEmpty())
             front = instance->arrayData + pidx;
 
     ScopedValue result(scope, front ? instance->getValue(front, instance->arrayAttributes ? instance->arrayAttributes[pidx] : Attr_Data) : Encode::undefined());
diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index a07e5dfe1d..0ccd8e6f62 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -304,14 +304,10 @@ Property *Object::__getOwnProperty__(uint index, PropertyAttributes *attrs)
     uint pidx = propertyIndexFromArrayIndex(index);
     if (pidx < UINT_MAX) {
         Property *p = arrayData + pidx;
-        if (!arrayAttributes || arrayAttributes[pidx].isData()) {
+        if (!p->value.isEmpty() && !(arrayAttributes && arrayAttributes[pidx].isGeneric())) {
             if (attrs)
                 *attrs = arrayAttributes ? arrayAttributes[pidx] : PropertyAttributes(Attr_Data);
             return p;
-        } else if (arrayAttributes[pidx].isAccessor()) {
-            if (attrs)
-                *attrs = arrayAttributes ? arrayAttributes[pidx] : PropertyAttributes(Attr_Accessor);
-            return p;
         }
     }
     if (isStringObject()) {
@@ -356,7 +352,7 @@ Property *Object::__getPropertyDescriptor__(uint index, PropertyAttributes *attr
         uint pidx = o->propertyIndexFromArrayIndex(index);
         if (pidx < UINT_MAX) {
             Property *p = o->arrayData + pidx;
-            if (!o->arrayAttributes || !o->arrayAttributes[pidx].isGeneric()) {
+            if (!p->value.isEmpty()) {
                 if (attrs)
                     *attrs = o->arrayAttributes ? o->arrayAttributes[pidx] : PropertyAttributes(Attr_Data);
                 return p;
@@ -448,7 +444,8 @@ PropertyAttributes Object::queryIndexed(const Managed *m, uint index)
     if (pidx < UINT_MAX) {
         if (o->arrayAttributes)
             return o->arrayAttributes[pidx];
-        return Attr_Data;
+        if (!o->arrayData[pidx].value.isEmpty())
+            return Attr_Data;
     }
     if (o->isStringObject()) {
         Property *p = static_cast<const StringObject *>(o)->getIndex(index);
@@ -578,7 +575,7 @@ Property *Object::advanceIterator(Managed *m, ObjectIterator *it, StringRef name
         Property *p = o->arrayData + pidx;
         PropertyAttributes a = o->arrayAttributes ? o->arrayAttributes[pidx] : PropertyAttributes(Attr_Data);
         ++it->arrayIndex;
-        if ((!o->arrayAttributes || !o->arrayAttributes[pidx].isGeneric())
+        if (!p->value.isEmpty()
             && (!(it->flags & ObjectIterator::EnumerableOnly) || a.isEnumerable())) {
             *index = it->arrayIndex - 1;
             if (attrs)
@@ -639,7 +636,7 @@ ReturnedValue Object::internalGetIndexed(uint index, bool *hasProperty)
     while (o) {
         uint pidx = o->propertyIndexFromArrayIndex(index);
         if (pidx < UINT_MAX) {
-            if (!o->arrayAttributes || !o->arrayAttributes[pidx].isGeneric()) {
+            if (!o->arrayData[pidx].value.isEmpty()) {
                 pd = o->arrayData + pidx;
                 if (o->arrayAttributes)
                     attrs = o->arrayAttributes[pidx];
@@ -757,13 +754,9 @@ void Object::internalPutIndexed(uint index, const ValueRef value)
     PropertyAttributes attrs;
 
     uint pidx = propertyIndexFromArrayIndex(index);
-    if (pidx < UINT_MAX) {
-        if (arrayAttributes && arrayAttributes[pidx].isGeneric()) {
-            pidx = UINT_MAX;
-        } else {
-            pd = arrayData + pidx;
-            attrs = arrayAttributes ? arrayAttributes[pidx] : PropertyAttributes(Attr_Data);
-        }
+    if (pidx < UINT_MAX && !arrayData[pidx].value.isEmpty()) {
+        pd = arrayData + pidx;
+        attrs = arrayAttributes ? arrayAttributes[pidx] : PropertyAttributes(Attr_Data);
     }
 
     if (!pd && isStringObject()) {
@@ -852,14 +845,13 @@ bool Object::internalDeleteIndexedProperty(uint index)
     uint pidx = propertyIndexFromArrayIndex(index);
     if (pidx == UINT_MAX)
         return true;
-    if (arrayAttributes && arrayAttributes[pidx].isGeneric())
+    if (arrayData[pidx].value.isEmpty())
         return true;
 
     if (!arrayAttributes || arrayAttributes[pidx].isConfigurable()) {
-        arrayData[pidx].value = Primitive::undefinedValue();
-        if (!arrayAttributes)
-            ensureArrayAttributes();
-        arrayAttributes[pidx].clear();
+        arrayData[pidx].value = Primitive::emptyValue();
+        if (arrayAttributes)
+            arrayAttributes[pidx].clear();
         if (sparseArray) {
             arrayData[pidx].value.int_32 = arrayFreeList;
             arrayFreeList = pidx;
@@ -951,7 +943,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, uint index, const Prop
     // Clause 1
     {
         uint pidx = propertyIndexFromArrayIndex(index);
-        if (pidx < UINT_MAX && (!arrayAttributes || !arrayAttributes[pidx].isGeneric()))
+        if (pidx < UINT_MAX && !arrayData[pidx].value.isEmpty())
             current = arrayData + pidx;
         if (!current && isStringObject())
             current = static_cast<StringObject *>(this)->getIndex(index);
@@ -1000,7 +992,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, Property *current, con
     }
 
     // clause 8
-    if (attrs.isGeneric())
+    if (attrs.isGeneric() || current->value.isEmpty())
         goto accept;
 
     // clause 9
@@ -1120,7 +1112,7 @@ ReturnedValue Object::arrayIndexOf(const ValueRef v, uint fromIndex, uint endInd
         Property *end = pd + endIndex;
         pd += fromIndex;
         while (pd < end) {
-            if (!arrayAttributes || !arrayAttributes[pd - arrayData].isGeneric()) {
+            if (!pd->value.isEmpty()) {
                 value = o->getValue(pd, arrayAttributes ? arrayAttributes[pd - arrayData] : Attr_Data);
                 if (__qmljs_strict_equal(value, v))
                     return Encode((uint)(pd - arrayData));
@@ -1156,8 +1148,8 @@ void Object::arrayConcat(const ArrayObject *other)
         int oldSize = arrayLength();
         arrayReserve(oldSize + other->arrayDataLen);
         if (oldSize > arrayDataLen) {
-            ensureArrayAttributes();
-            std::fill(arrayAttributes + arrayDataLen, arrayAttributes + oldSize, PropertyAttributes());
+            for (int i = arrayDataLen; i < oldSize; ++i)
+                arrayData[i].value = Primitive::emptyValue();
         }
         if (other->arrayAttributes) {
             for (int i = 0; i < other->arrayDataLen; ++i) {
@@ -1166,10 +1158,8 @@ void Object::arrayConcat(const ArrayObject *other)
                 arrayDataLen = oldSize + i + 1;
                 if (arrayAttributes)
                     arrayAttributes[oldSize + i] = Attr_Data;
-                if (!exists) {
-                    ensureArrayAttributes();
-                    arrayAttributes[oldSize + i].clear();
-                }
+                if (!exists)
+                    arrayData[oldSize + i].value = Primitive::emptyValue();
             }
         } else {
             arrayDataLen = oldSize + other->arrayDataLen;
@@ -1200,13 +1190,16 @@ void Object::arraySort(ExecutionContext *context, ObjectRef thisObject, const Va
     // into data properties and then sort. This is in line with the sentence above.
     if (arrayAttributes) {
         for (uint i = 0; i < len; i++) {
-            if (arrayAttributes[i].isGeneric()) {
+            if ((arrayAttributes && arrayAttributes[i].isGeneric()) || arrayData[i].value.isEmpty()) {
                 while (--len > i)
-                    if (!arrayAttributes[len].isGeneric())
+                    if (!((arrayAttributes && arrayAttributes[len].isGeneric())|| arrayData[len].value.isEmpty()))
                         break;
                 arrayData[i].value = getValue(arrayData + len, arrayAttributes[len]);
-                arrayAttributes[i] = Attr_Data;
-                arrayAttributes[len].clear();
+                arrayData[len].value = Primitive::emptyValue();
+                if (arrayAttributes) {
+                    arrayAttributes[i] = Attr_Data;
+                    arrayAttributes[len].clear();
+                }
             } else if (arrayAttributes[i].isAccessor()) {
                 arrayData[i].value = getValue(arrayData + i, arrayAttributes[i]);
                 arrayAttributes[i] = Attr_Data;
@@ -1235,7 +1228,7 @@ void Object::initSparse()
     if (!sparseArray) {
         sparseArray = new SparseArray;
         for (int i = 0; i < arrayDataLen; ++i) {
-            if (!arrayAttributes || !arrayAttributes[i].isGeneric()) {
+            if (!((arrayAttributes && arrayAttributes[i].isGeneric()) || arrayData[i].value.isEmpty())) {
                 SparseArrayNode *n = sparseArray->insert(i);
                 n->value = i + arrayOffset;
             }
diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index 27ecf8bc58..8807f60194 100644
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -422,9 +422,12 @@ inline Property *Object::arrayInsert(uint index, PropertyAttributes attributes)
         if (index >= arrayAlloc)
             arrayReserve(index + 1);
         if (index >= arrayDataLen) {
-            ensureArrayAttributes();
-            for (uint i = arrayDataLen; i < index; ++i)
-                arrayAttributes[i].clear();
+            // mark possible hole in the array
+            for (uint i = arrayDataLen; i < index; ++i) {
+                arrayData[i].value = Primitive::emptyValue();
+                if (arrayAttributes)
+                    arrayAttributes[i].clear();
+            }
             arrayDataLen = index + 1;
         }
         pd = arrayData + index;
diff --git a/src/qml/jsruntime/qv4objectproto.cpp b/src/qml/jsruntime/qv4objectproto.cpp
index 21cbc8d7fd..7cfda589e5 100644
--- a/src/qml/jsruntime/qv4objectproto.cpp
+++ b/src/qml/jsruntime/qv4objectproto.cpp
@@ -259,7 +259,7 @@ ReturnedValue ObjectPrototype::method_seal(SimpleCallContext *ctx)
 
     o->ensureArrayAttributes();
     for (uint i = 0; i < o->arrayDataLen; ++i) {
-        if (!o->arrayAttributes[i].isGeneric())
+        if (!(o->arrayAttributes[i].isGeneric() || o->arrayData[i].value.isEmpty()))
             o->arrayAttributes[i].setConfigurable(false);
     }
 
@@ -279,7 +279,7 @@ ReturnedValue ObjectPrototype::method_freeze(SimpleCallContext *ctx)
 
     o->ensureArrayAttributes();
     for (uint i = 0; i < o->arrayDataLen; ++i) {
-        if (!o->arrayAttributes[i].isGeneric())
+        if (!(o->arrayAttributes[i].isGeneric() || o->arrayData[i].value.isEmpty()))
             o->arrayAttributes[i].setConfigurable(false);
         if (o->arrayAttributes[i].isData())
             o->arrayAttributes[i].setWritable(false);
@@ -318,7 +318,7 @@ ReturnedValue ObjectPrototype::method_isSealed(SimpleCallContext *ctx)
         return Encode(false);
 
     for (uint i = 0; i < o->arrayDataLen; ++i) {
-        if (!o->arrayAttributes[i].isGeneric())
+        if (!(o->arrayAttributes[i].isGeneric() || o->arrayData[i].value.isEmpty()))
             if (o->arrayAttributes[i].isConfigurable())
                 return Encode(false);
     }
@@ -346,7 +346,7 @@ ReturnedValue ObjectPrototype::method_isFrozen(SimpleCallContext *ctx)
         return Encode(false);
 
     for (uint i = 0; i < o->arrayDataLen; ++i) {
-        if (!o->arrayAttributes[i].isGeneric())
+        if (!(o->arrayAttributes[i].isGeneric() || o->arrayData[i].value.isEmpty()))
             if (o->arrayAttributes[i].isConfigurable() || o->arrayAttributes[i].isWritable())
                 return Encode(false);
     }
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index 6b3afcc300..6f914fa3c2 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -500,7 +500,8 @@ ReturnedValue __qmljs_get_element(ExecutionContext *ctx, const ValueRef object,
         uint pidx = o->propertyIndexFromArrayIndex(idx);
         if (pidx < UINT_MAX) {
             if (!o->arrayAttributes || o->arrayAttributes[pidx].isData()) {
-                return o->arrayData[pidx].value.asReturnedValue();
+                if (!o->arrayData[pidx].value.isEmpty())
+                    return o->arrayData[pidx].value.asReturnedValue();
             }
         }
 
@@ -996,13 +997,7 @@ ReturnedValue __qmljs_builtin_define_array(ExecutionContext *ctx, Value *values,
         a->arrayDataLen = length;
         Property *pd = a->arrayData;
         for (uint i = 0; i < length; ++i) {
-            if (values[i].isEmpty()) {
-                a->ensureArrayAttributes();
-                pd->value = Primitive::undefinedValue();
-                a->arrayAttributes[i].clear();
-            } else {
-                pd->value = values[i];
-            }
+            pd->value = values[i];
             ++pd;
         }
         a->setArrayLengthUnchecked(length);