[DFG] Fold GetByVal if the indexed value is non configurable and non writable
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Jul 2018 16:54:38 +0000 (16:54 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Jul 2018 16:54:38 +0000 (16:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186462

Reviewed by Saam Barati.

JSTests:

* stress/folding-get-by-val-with-read-only-dont-delete-object.js: Added.
(shouldBe):
(test1):
(test2):
(test3):
(test4):
(test5):
* stress/folding-get-by-val-with-read-only-dont-delete-runtime-array.js: Added.
(shouldBe):
(test1):
(test2):
(test5):
* stress/folding-get-by-val-with-read-only-dont-delete.js: Added.
(shouldBe):
(test1):
(test2):
(test3):
(test4):
(test5):

Source/JavaScriptCore:

Non-special DontDelete | ReadOnly properties mean that it won't be changed. If DFG AI can retrieve this
property, AI can fold it into a constant. This type of property can be seen when we use ES6 tagged templates.
Tagged templates' callsite includes indexed properties whose attributes are DontDelete | ReadOnly.

This patch attempts to fold such properties into constant in DFG AI. The challenge is that DFG AI runs
concurrently with the mutator thread. In this patch, we insert WTF::storeStoreFence between value setting
and attributes setting. The attributes must be set after the corresponding value is set. If the loaded
attributes (with WTF::loadLoadFence) include DontDelete | ReadOnly, it means the given value won't be
changed and we can safely use it. We arrange our existing code to use this protocol.

Since GetByVal folding requires the correct Structure & Butterfly pairs, it is only enabled in x86 architecture
since it is TSO. So, our WTF::storeStoreFence in SparseArrayValueMap is also emitted only in x86.

This patch improves SixSpeed/template_string_tag.es6.

                                  baseline                  patched

template_string_tag.es6      237.0301+-4.8374     ^      9.8779+-0.3628        ^ definitely 23.9960x faster

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* runtime/JSArray.cpp:
(JSC::JSArray::setLengthWithArrayStorage):
* runtime/JSObject.cpp:
(JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
(JSC::JSObject::deletePropertyByIndex):
(JSC::JSObject::getOwnPropertyNames):
(JSC::putIndexedDescriptor):
(JSC::JSObject::defineOwnIndexedProperty):
(JSC::JSObject::attemptToInterceptPutByIndexOnHoleForPrototype):
(JSC::JSObject::putIndexedDescriptor): Deleted.
* runtime/JSObject.h:
* runtime/SparseArrayValueMap.cpp:
(JSC::SparseArrayValueMap::SparseArrayValueMap):
(JSC::SparseArrayValueMap::add):
(JSC::SparseArrayValueMap::putDirect):
(JSC::SparseArrayValueMap::getConcurrently):
(JSC::SparseArrayEntry::get const):
(JSC::SparseArrayEntry::getConcurrently const):
(JSC::SparseArrayEntry::put):
(JSC::SparseArrayEntry::getNonSparseMode const):
(JSC::SparseArrayValueMap::visitChildren):
(JSC::SparseArrayValueMap::~SparseArrayValueMap): Deleted.
* runtime/SparseArrayValueMap.h:
(JSC::SparseArrayEntry::SparseArrayEntry):
(JSC::SparseArrayEntry::attributes const):
(JSC::SparseArrayEntry::forceSet):
(JSC::SparseArrayEntry::asValue):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234089 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/folding-get-by-val-with-read-only-dont-delete-object.js [new file with mode: 0644]
JSTests/stress/folding-get-by-val-with-read-only-dont-delete-runtime-array.js [new file with mode: 0644]
JSTests/stress/folding-get-by-val-with-read-only-dont-delete.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp
Source/JavaScriptCore/runtime/SparseArrayValueMap.h

index 77719b4..dcda352 100644 (file)
@@ -1,3 +1,30 @@
+2018-07-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Fold GetByVal if the indexed value is non configurable and non writable
+        https://bugs.webkit.org/show_bug.cgi?id=186462
+
+        Reviewed by Saam Barati.
+
+        * stress/folding-get-by-val-with-read-only-dont-delete-object.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        (test3):
+        (test4):
+        (test5):
+        * stress/folding-get-by-val-with-read-only-dont-delete-runtime-array.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        (test5):
+        * stress/folding-get-by-val-with-read-only-dont-delete.js: Added.
+        (shouldBe):
+        (test1):
+        (test2):
+        (test3):
+        (test4):
+        (test5):
+
 2018-06-02  Filip Pizlo  <fpizlo@apple.com>
 
         We should support CreateThis in the FTL
diff --git a/JSTests/stress/folding-get-by-val-with-read-only-dont-delete-object.js b/JSTests/stress/folding-get-by-val-with-read-only-dont-delete-object.js
new file mode 100644 (file)
index 0000000..50fdf9f
--- /dev/null
@@ -0,0 +1,60 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+var array1 = {0:0, 1:1, 2:2, 3:3, 4:4, 5:5};
+var array2 = {0:"Hello", 1:"World", 2:"Cocoa"};
+Object.freeze(array1);
+Object.freeze(array2);
+
+function test1()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5];
+}
+noInline(test1);
+
+function test2()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5] + (array1[6] | 0);
+}
+noInline(test2);
+
+function test3()
+{
+    return array2[0] + array2[1] + array2[2];
+}
+noInline(test3);
+
+var array3 = {};
+Object.defineProperty(array3, 0, {
+    get() { return 42; }
+});
+Object.defineProperty(array3, 1, {
+    get() { return 42; }
+});
+Object.freeze(array3);
+
+function test4()
+{
+    return array3[0] + array3[1];
+}
+noInline(test4);
+
+var array4 = {0:0, 1:1, 2:2, 3:3, 4:4, 5:5};
+Object.seal(array4);
+
+function test5()
+{
+    return array4[0] + array4[1] + array4[2] + array4[3] + array4[4] + array4[5];
+}
+noInline(test5);
+
+for (var i = 0; i < 1e5; ++i) {
+    shouldBe(test1(), 15);
+    shouldBe(test2(), 15);
+    shouldBe(test3(), `HelloWorldCocoa`);
+    shouldBe(test4(), 84);
+    shouldBe(test5(), 15);
+}
diff --git a/JSTests/stress/folding-get-by-val-with-read-only-dont-delete-runtime-array.js b/JSTests/stress/folding-get-by-val-with-read-only-dont-delete-runtime-array.js
new file mode 100644 (file)
index 0000000..3739b42
--- /dev/null
@@ -0,0 +1,35 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+var array1 = $vm.createRuntimeArray(0, 1, 2, 3, 4, 5);
+Object.freeze(array1);
+
+function test1()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5];
+}
+noInline(test1);
+
+function test2()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5] + (array1[6] | 0);
+}
+noInline(test2);
+
+var array4 = $vm.createRuntimeArray(0, 1, 2, 3, 4, 5);
+Object.seal(array4);
+
+function test5()
+{
+    return array4[0] + array4[1] + array4[2] + array4[3] + array4[4] + array4[5];
+}
+noInline(test5);
+
+for (var i = 0; i < 1e5; ++i) {
+    shouldBe(test1(), 15);
+    shouldBe(test2(), 15);
+    shouldBe(test5(), 15);
+}
diff --git a/JSTests/stress/folding-get-by-val-with-read-only-dont-delete.js b/JSTests/stress/folding-get-by-val-with-read-only-dont-delete.js
new file mode 100644 (file)
index 0000000..92fbcbf
--- /dev/null
@@ -0,0 +1,60 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+var array1 = [0, 1, 2, 3, 4, 5];
+var array2 = ["Hello", "World", "Cocoa"];
+Object.freeze(array1);
+Object.freeze(array2);
+
+function test1()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5];
+}
+noInline(test1);
+
+function test2()
+{
+    return array1[0] + array1[1] + array1[2] + array1[3] + array1[4] + array1[5] + (array1[6] | 0);
+}
+noInline(test2);
+
+function test3()
+{
+    return array2[0] + array2[1] + array2[2];
+}
+noInline(test3);
+
+var array3 = [];
+Object.defineProperty(array3, 0, {
+    get() { return 42; }
+});
+Object.defineProperty(array3, 1, {
+    get() { return 42; }
+});
+Object.freeze(array3);
+
+function test4()
+{
+    return array3[0] + array3[1];
+}
+noInline(test4);
+
+var array4 = [0, 1, 2, 3, 4, 5];
+Object.seal(array4);
+
+function test5()
+{
+    return array4[0] + array4[1] + array4[2] + array4[3] + array4[4] + array4[5];
+}
+noInline(test5);
+
+for (var i = 0; i < 1e5; ++i) {
+    shouldBe(test1(), 15);
+    shouldBe(test2(), 15);
+    shouldBe(test3(), `HelloWorldCocoa`);
+    shouldBe(test4(), 84);
+    shouldBe(test5(), 15);
+}
index ffd44b1..7a7797b 100644 (file)
@@ -1,3 +1,59 @@
+2018-07-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DFG] Fold GetByVal if the indexed value is non configurable and non writable
+        https://bugs.webkit.org/show_bug.cgi?id=186462
+
+        Reviewed by Saam Barati.
+
+        Non-special DontDelete | ReadOnly properties mean that it won't be changed. If DFG AI can retrieve this
+        property, AI can fold it into a constant. This type of property can be seen when we use ES6 tagged templates.
+        Tagged templates' callsite includes indexed properties whose attributes are DontDelete | ReadOnly.
+
+        This patch attempts to fold such properties into constant in DFG AI. The challenge is that DFG AI runs
+        concurrently with the mutator thread. In this patch, we insert WTF::storeStoreFence between value setting
+        and attributes setting. The attributes must be set after the corresponding value is set. If the loaded
+        attributes (with WTF::loadLoadFence) include DontDelete | ReadOnly, it means the given value won't be
+        changed and we can safely use it. We arrange our existing code to use this protocol.
+
+        Since GetByVal folding requires the correct Structure & Butterfly pairs, it is only enabled in x86 architecture
+        since it is TSO. So, our WTF::storeStoreFence in SparseArrayValueMap is also emitted only in x86.
+
+        This patch improves SixSpeed/template_string_tag.es6.
+
+                                          baseline                  patched
+
+        template_string_tag.es6      237.0301+-4.8374     ^      9.8779+-0.3628        ^ definitely 23.9960x faster
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::setLengthWithArrayStorage):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
+        (JSC::JSObject::deletePropertyByIndex):
+        (JSC::JSObject::getOwnPropertyNames):
+        (JSC::putIndexedDescriptor):
+        (JSC::JSObject::defineOwnIndexedProperty):
+        (JSC::JSObject::attemptToInterceptPutByIndexOnHoleForPrototype):
+        (JSC::JSObject::putIndexedDescriptor): Deleted.
+        * runtime/JSObject.h:
+        * runtime/SparseArrayValueMap.cpp:
+        (JSC::SparseArrayValueMap::SparseArrayValueMap):
+        (JSC::SparseArrayValueMap::add):
+        (JSC::SparseArrayValueMap::putDirect):
+        (JSC::SparseArrayValueMap::getConcurrently):
+        (JSC::SparseArrayEntry::get const):
+        (JSC::SparseArrayEntry::getConcurrently const):
+        (JSC::SparseArrayEntry::put):
+        (JSC::SparseArrayEntry::getNonSparseMode const):
+        (JSC::SparseArrayValueMap::visitChildren):
+        (JSC::SparseArrayValueMap::~SparseArrayValueMap): Deleted.
+        * runtime/SparseArrayValueMap.h:
+        (JSC::SparseArrayEntry::SparseArrayEntry):
+        (JSC::SparseArrayEntry::attributes const):
+        (JSC::SparseArrayEntry::forceSet):
+        (JSC::SparseArrayEntry::asValue):
+
 2018-06-02  Filip Pizlo  <fpizlo@apple.com>
 
         We should support CreateThis in the FTL
index aec920b..ea425d1 100644 (file)
@@ -1818,15 +1818,13 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     case AtomicsSub:
     case AtomicsXor: {
         if (node->op() == GetByVal) {
-            auto foldGetByValOnCoWArray = [&] (Edge& arrayEdge, Edge& indexEdge) {
+            auto foldGetByValOnConstantProperty = [&] (Edge& arrayEdge, Edge& indexEdge) {
                 // FIXME: We can expand this for non x86 environments.
                 // https://bugs.webkit.org/show_bug.cgi?id=134641
                 if (!isX86())
                     return false;
 
                 AbstractValue& arrayValue = forNode(arrayEdge);
-                if (node->arrayMode().arrayClass() != Array::OriginalCopyOnWriteArray)
-                    return false;
 
                 // Check the structure set is finite. This means that this constant's structure is watched and guaranteed the one of this set.
                 // When the structure is changed, this code should be invalidated. This is important since the following code relies on the
@@ -1863,13 +1861,71 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
                 if (structureIDEarly != structureIDLate)
                     return false;
 
-                if (!isCopyOnWrite(m_vm.getStructure(structureIDLate)->indexingMode()))
+                Structure* structure = m_vm.getStructure(structureIDLate);
+                if (node->arrayMode().arrayClass() == Array::OriginalCopyOnWriteArray) {
+                    if (!isCopyOnWrite(structure->indexingMode()))
+                        return false;
+
+                    JSImmutableButterfly* immutableButterfly = JSImmutableButterfly::fromButterfly(butterfly);
+                    if (index < immutableButterfly->length()) {
+                        JSValue value = immutableButterfly->get(index);
+                        ASSERT(value);
+                        if (value.isCell())
+                            setConstant(node, *m_graph.freeze(value.asCell()));
+                        else
+                            setConstant(node, value);
+                        return true;
+                    }
+
+                    if (node->arrayMode().isOutOfBounds()) {
+                        JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+                        Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(m_vm);
+                        Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(m_vm);
+                        if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                            && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
+                            && globalObject->arrayPrototypeChainIsSane()) {
+                            m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+                            m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+                            didFoldClobberWorld();
+                            // Note that Array::Double and Array::Int32 return JSValue if array mode is OutOfBounds.
+                            setConstant(node, jsUndefined());
+                            return true;
+                        }
+                    }
                     return false;
+                }
+
+                if (node->arrayMode().type() == Array::ArrayStorage || node->arrayMode().type() == Array::SlowPutArrayStorage) {
+                    if (!hasAnyArrayStorage(structure->indexingMode()))
+                        return false;
+
+                    if (structure->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
+                        return false;
+
+                    JSValue value;
+                    {
+                        // ArrayStorage's Butterfly can be half-broken state.
+                        auto locker = holdLock(array->cellLock());
+
+                        ArrayStorage* storage = butterfly->arrayStorage();
+                        if (index >= storage->length())
+                            return false;
+
+                        if (index < storage->vectorLength())
+                            return false;
+
+                        SparseArrayValueMap* map = storage->m_sparseMap.get();
+                        if (!map)
+                            return false;
+
+                        value = map->getConcurrently(index);
+                    }
+                    if (!value)
+                        return false;
+
+                    if (node->arrayMode().isOutOfBounds())
+                        didFoldClobberWorld();
 
-                JSImmutableButterfly* immutableButterfly = JSImmutableButterfly::fromButterfly(butterfly);
-                if (index < immutableButterfly->length()) {
-                    JSValue value = immutableButterfly->get(index);
-                    ASSERT(value);
                     if (value.isCell())
                         setConstant(node, *m_graph.freeze(value.asCell()));
                     else
@@ -1877,25 +1933,10 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
                     return true;
                 }
 
-                if (node->arrayMode().isOutOfBounds()) {
-                    JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
-                    Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure(m_vm);
-                    Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure(m_vm);
-                    if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
-                        && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
-                        && globalObject->arrayPrototypeChainIsSane()) {
-                        m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
-                        m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
-                        didFoldClobberWorld();
-                        // Note that Array::Double and Array::Int32 return JSValue if array mode is OutOfBounds.
-                        setConstant(node, jsUndefined());
-                        return true;
-                    }
-                }
                 return false;
             };
 
-            if (foldGetByValOnCoWArray(m_graph.child(node, 0), m_graph.child(node, 1)))
+            if (foldGetByValOnConstantProperty(m_graph.child(node, 0), m_graph.child(node, 1)))
                 break;
         }
 
index b66b7c9..53b157a 100644 (file)
@@ -485,7 +485,7 @@ bool JSArray::setLengthWithArrayStorage(ExecState* exec, unsigned newLength, boo
                     unsigned index = keys[--i];
                     SparseArrayValueMap::iterator it = map->find(index);
                     ASSERT(it != map->notFound());
-                    if (it->value.attributes & PropertyAttribute::DontDelete) {
+                    if (it->value.attributes() & PropertyAttribute::DontDelete) {
                         storage->setLength(index + 1);
                         return typeError(exec, scope, throwException, UnableToDeletePropertyError);
                     }
index bceb458..f9ed6ea 100644 (file)
@@ -966,7 +966,7 @@ ArrayStorage* JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists
         // This will always be a new entry in the map, so no need to check we can write,
         // and attributes are default so no need to set them.
         if (value)
-            map->add(this, i).iterator->value.set(vm, map, value);
+            map->add(this, i).iterator->value.forceSet(vm, map, value, 0);
     }
 
     DeferGC deferGC(vm.heap);
@@ -2025,7 +2025,7 @@ bool JSObject::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned i)
         } else if (SparseArrayValueMap* map = storage->m_sparseMap.get()) {
             SparseArrayValueMap::iterator it = map->find(i);
             if (it != map->notFound()) {
-                if (it->value.attributes & PropertyAttribute::DontDelete)
+                if (it->value.attributes() & PropertyAttribute::DontDelete)
                     return false;
                 map->remove(it);
             }
@@ -2350,7 +2350,7 @@ void JSObject::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNa
                 
                 SparseArrayValueMap::const_iterator end = map->end();
                 for (SparseArrayValueMap::const_iterator it = map->begin(); it != end; ++it) {
-                    if (mode.includeDontEnumProperties() || !(it->value.attributes & PropertyAttribute::DontEnum))
+                    if (mode.includeDontEnumProperties() || !(it->value.attributes() & PropertyAttribute::DontEnum))
                         keys.uncheckedAppend(static_cast<unsigned>(it->key));
                 }
                 
@@ -2491,17 +2491,18 @@ NEVER_INLINE void JSObject::fillGetterPropertySlot(VM& vm, PropertySlot& slot, J
     slot.setCacheableGetterSlot(this, attributes, jsCast<GetterSetter*>(getterSetter), offset);
 }
 
-bool JSObject::putIndexedDescriptor(ExecState* exec, SparseArrayEntry* entryInMap, const PropertyDescriptor& descriptor, PropertyDescriptor& oldDescriptor)
+static bool putIndexedDescriptor(ExecState* exec, SparseArrayValueMap* map, SparseArrayEntry* entryInMap, const PropertyDescriptor& descriptor, PropertyDescriptor& oldDescriptor)
 {
     VM& vm = exec->vm();
-    auto map = m_butterfly->arrayStorage()->m_sparseMap.get();
 
     if (descriptor.isDataDescriptor()) {
+        unsigned attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~PropertyAttribute::Accessor;
         if (descriptor.value())
-            entryInMap->set(vm, map, descriptor.value());
+            entryInMap->forceSet(vm, map, descriptor.value(), attributes);
         else if (oldDescriptor.isAccessorDescriptor())
-            entryInMap->set(vm, map, jsUndefined());
-        entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~PropertyAttribute::Accessor;
+            entryInMap->forceSet(vm, map, jsUndefined(), attributes);
+        else
+            entryInMap->forceSet(attributes);
         return true;
     }
 
@@ -2518,14 +2519,12 @@ bool JSObject::putIndexedDescriptor(ExecState* exec, SparseArrayEntry* entryInMa
             setter = oldDescriptor.setterObject();
 
         GetterSetter* accessor = GetterSetter::create(vm, exec->lexicalGlobalObject(), getter, setter);
-
-        entryInMap->set(vm, map, accessor);
-        entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~PropertyAttribute::ReadOnly;
+        entryInMap->forceSet(vm, map, accessor, descriptor.attributesOverridingCurrent(oldDescriptor) & ~PropertyAttribute::ReadOnly);
         return true;
     }
 
     ASSERT(descriptor.isGenericDescriptor());
-    entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor);
+    entryInMap->forceSet(descriptor.attributesOverridingCurrent(oldDescriptor));
     return true;
 }
 
@@ -2590,12 +2589,8 @@ bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const P
         // is set to its default value.
         // 4.c. Return true.
 
-        PropertyDescriptor defaults;
-        entryInMap->setWithoutWriteBarrier(jsUndefined());
-        entryInMap->attributes = PropertyAttribute::DontDelete | PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly;
-        entryInMap->get(defaults);
-
-        putIndexedDescriptor(exec, entryInMap, descriptor, defaults);
+        PropertyDescriptor defaults(jsUndefined(), PropertyAttribute::DontDelete | PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
+        putIndexedDescriptor(exec, map, entryInMap, descriptor, defaults);
         Butterfly* butterfly = m_butterfly.get();
         if (index >= butterfly->arrayStorage()->length())
             butterfly->arrayStorage()->setLength(index + 1);
@@ -2663,7 +2658,7 @@ bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const P
     }
 
     // 12. For each attribute field of Desc that is present, set the correspondingly named attribute of the property named P of object O to the value of the field.
-    putIndexedDescriptor(exec, entryInMap, descriptor, current);
+    putIndexedDescriptor(exec, map, entryInMap, descriptor, current);
     // 13. Return true.
     return true;
 }
@@ -2693,7 +2688,7 @@ bool JSObject::attemptToInterceptPutByIndexOnHoleForPrototype(ExecState* exec, J
         ArrayStorage* storage = current->arrayStorageOrNull();
         if (storage && storage->m_sparseMap) {
             SparseArrayValueMap::iterator iter = storage->m_sparseMap->find(i);
-            if (iter != storage->m_sparseMap->notFound() && (iter->value.attributes & (PropertyAttribute::Accessor | PropertyAttribute::ReadOnly))) {
+            if (iter != storage->m_sparseMap->notFound() && (iter->value.attributes() & (PropertyAttribute::Accessor | PropertyAttribute::ReadOnly))) {
                 putResult = iter->value.put(exec, thisValue, storage->m_sparseMap.get(), value, shouldThrow);
                 return true;
             }
index c11d050..59c5138 100644 (file)
@@ -1044,8 +1044,6 @@ private:
     };
     std::optional<PropertyHashEntry> findPropertyHashEntry(VM&, PropertyName) const;
         
-    bool putIndexedDescriptor(ExecState*, SparseArrayEntry*, const PropertyDescriptor&, PropertyDescriptor& old);
-        
     bool putByIndexBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, bool shouldThrow);
     bool putDirectIndexBeyondVectorLengthWithArrayStorage(ExecState*, unsigned propertyName, JSValue, unsigned attributes, PutDirectIndexMode, ArrayStorage*);
     JS_EXPORT_PRIVATE bool putDirectIndexSlowOrBeyondVectorLength(ExecState*, unsigned propertyName, JSValue, unsigned attributes, PutDirectIndexMode);
index a786dcb..86acde5 100644 (file)
@@ -41,12 +41,6 @@ const ClassInfo SparseArrayValueMap::s_info = { "SparseArrayValueMap", nullptr,
 
 SparseArrayValueMap::SparseArrayValueMap(VM& vm)
     : Base(vm, vm.sparseArrayValueMapStructure.get())
-    , m_flags(Normal)
-    , m_reportedCapacity(0)
-{
-}
-
-SparseArrayValueMap::~SparseArrayValueMap()
 {
 }
 
@@ -78,10 +72,7 @@ SparseArrayValueMap::AddResult SparseArrayValueMap::add(JSObject* array, unsigne
     size_t capacity;
     {
         auto locker = holdLock(cellLock());
-        SparseArrayEntry entry;
-        entry.setWithoutWriteBarrier(jsUndefined());
-        
-        result = m_map.add(i, entry);
+        result = m_map.add(i, SparseArrayEntry());
         capacity = m_map.capacity();
     }
     if (capacity > m_reportedCapacity) {
@@ -145,30 +136,59 @@ bool SparseArrayValueMap::putDirect(ExecState* exec, JSObject* array, unsigned i
         return typeError(exec, scope, shouldThrow, NonExtensibleObjectPropertyDefineError);
     }
 
-    if (entry.attributes & PropertyAttribute::ReadOnly)
+    if (entry.attributes() & PropertyAttribute::ReadOnly)
         return typeError(exec, scope, shouldThrow, ReadonlyPropertyWriteError);
 
-    entry.attributes = attributes;
-    entry.set(vm, this, value);
+    entry.forceSet(vm, this, value, attributes);
     return true;
 }
 
+JSValue SparseArrayValueMap::getConcurrently(unsigned i)
+{
+    auto locker = holdLock(cellLock());
+    auto iterator = m_map.find(i);
+    if (iterator == m_map.end())
+        return JSValue();
+    return iterator->value.getConcurrently();
+}
+
 void SparseArrayEntry::get(JSObject* thisObject, PropertySlot& slot) const
 {
     JSValue value = Base::get();
     ASSERT(value);
 
     if (LIKELY(!value.isGetterSetter())) {
-        slot.setValue(thisObject, attributes, value);
+        slot.setValue(thisObject, m_attributes, value);
         return;
     }
 
-    slot.setGetterSlot(thisObject, attributes, jsCast<GetterSetter*>(value));
+    slot.setGetterSlot(thisObject, m_attributes, jsCast<GetterSetter*>(value));
 }
 
 void SparseArrayEntry::get(PropertyDescriptor& descriptor) const
 {
-    descriptor.setDescriptor(Base::get(), attributes);
+    descriptor.setDescriptor(Base::get(), m_attributes);
+}
+
+JSValue SparseArrayEntry::getConcurrently() const
+{
+    // These attributes and value can be updated while executing getConcurrently.
+    // But this is OK since attributes should be never weaken once it gets DontDelete and ReadOnly.
+    // By emitting store-store-fence and load-load-fence between value setting and attributes setting,
+    // we can ensure that the value is what we want once the attributes get ReadOnly & DontDelete:
+    // once attributes get this state, the value should not be changed.
+    unsigned attributes = m_attributes;
+    Dependency attributesDependency = Dependency::fence(attributes);
+    if (attributes & PropertyAttribute::Accessor)
+        return JSValue();
+
+    if (!(attributes & PropertyAttribute::ReadOnly))
+        return JSValue();
+
+    if (!(attributes & PropertyAttribute::DontDelete))
+        return JSValue();
+
+    return attributesDependency.consume(this)->Base::get();
 }
 
 bool SparseArrayEntry::put(ExecState* exec, JSValue thisValue, SparseArrayValueMap* map, JSValue value, bool shouldThrow)
@@ -176,8 +196,8 @@ bool SparseArrayEntry::put(ExecState* exec, JSValue thisValue, SparseArrayValueM
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    if (!(attributes & PropertyAttribute::Accessor)) {
-        if (attributes & PropertyAttribute::ReadOnly)
+    if (!(m_attributes & PropertyAttribute::Accessor)) {
+        if (m_attributes & PropertyAttribute::ReadOnly)
             return typeError(exec, scope, shouldThrow, ReadonlyPropertyWriteError);
 
         set(vm, map, value);
@@ -190,7 +210,7 @@ bool SparseArrayEntry::put(ExecState* exec, JSValue thisValue, SparseArrayValueM
 
 JSValue SparseArrayEntry::getNonSparseMode() const
 {
-    ASSERT(!attributes);
+    ASSERT(!m_attributes);
     return Base::get();
 }
 
@@ -202,7 +222,7 @@ void SparseArrayValueMap::visitChildren(JSCell* thisObject, SlotVisitor& visitor
     SparseArrayValueMap* thisMap = jsCast<SparseArrayValueMap*>(thisObject);
     iterator end = thisMap->m_map.end();
     for (iterator it = thisMap->m_map.begin(); it != end; ++it)
-        visitor.append(it->value);
+        visitor.append(it->value.asValue());
 }
 
 } // namespace JSC
index aa1ac22..771097b 100644 (file)
@@ -36,17 +36,43 @@ namespace JSC {
 
 class SparseArrayValueMap;
 
-struct SparseArrayEntry : public WriteBarrier<Unknown> {
-    typedef WriteBarrier<Unknown> Base;
+class SparseArrayEntry : private WriteBarrier<Unknown> {
+public:
+    using Base = WriteBarrier<Unknown>;
 
-    SparseArrayEntry() : attributes(0) { }
+    SparseArrayEntry()
+    {
+        Base::setWithoutWriteBarrier(jsUndefined());
+    }
 
     void get(JSObject*, PropertySlot&) const;
     void get(PropertyDescriptor&) const;
     bool put(ExecState*, JSValue thisValue, SparseArrayValueMap*, JSValue, bool shouldThrow);
     JSValue getNonSparseMode() const;
+    JSValue getConcurrently() const;
+
+    unsigned attributes() const { return m_attributes; }
+
+    void forceSet(unsigned attributes)
+    {
+        // FIXME: We can expand this for non x86 environments. Currently, loading ReadOnly | DontDelete property
+        // from compiler thread is only supported in X86 architecture because of its TSO nature.
+        // https://bugs.webkit.org/show_bug.cgi?id=134641
+        if (isX86())
+            WTF::storeStoreFence();
+        m_attributes = attributes;
+    }
 
-    unsigned attributes;
+    void forceSet(VM& vm, JSCell* map, JSValue value, unsigned attributes)
+    {
+        Base::set(vm, map, value);
+        forceSet(attributes);
+    }
+
+    WriteBarrier<Unknown>& asValue() { return *this; }
+
+private:
+    unsigned m_attributes { 0 };
 };
 
 class SparseArrayValueMap final : public JSCell {
@@ -64,7 +90,6 @@ private:
     };
 
     SparseArrayValueMap(VM&);
-    ~SparseArrayValueMap();
     
     void finishCreation(VM&);
 
@@ -113,6 +138,8 @@ public:
     void remove(iterator it);
     void remove(unsigned i);
 
+    JSValue getConcurrently(unsigned index);
+
     // These methods do not mutate the contents of the map.
     iterator notFound() { return m_map.end(); }
     bool isEmpty() const { return m_map.isEmpty(); }
@@ -124,8 +151,8 @@ public:
 
 private:
     Map m_map;
-    Flags m_flags;
-    size_t m_reportedCapacity;
+    Flags m_flags { Normal };
+    size_t m_reportedCapacity { 0 };
 };
 
 } // namespace JSC