Changeset 234089 in webkit for trunk/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp

Timestamp:

Jul 22, 2018, 9:54:38 AM (7 years ago)

Author:

Yusuke Suzuki

Message:

[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.

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):

File:

• trunk/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp (modified) (7 diffs)

trunk/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp

@@ -42 +42 @@
 SparseArrayValueMap::SparseArrayValueMap(VM& vm)
     : Base(vm, vm.sparseArrayValueMapStructure.get())
-    , m_flags(Normal)
-    , m_reportedCapacity(0)
-{
-}
-
-SparseArrayValueMap::~SparseArrayValueMap()
 {
 }
@@ -79 +73 @@
     {
         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();
     }
@@ -146 +137 @@
     }
 
-    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();
 }
 
@@ -160 +159 @@
 
     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();
 }
 
@@ -177 +197 @@
     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);
 
@@ -191 +211 @@
 JSValue SparseArrayEntry::getNonSparseMode() const
 {
-    ASSERT(!attributes);
+    ASSERT(!m_attributes);
     return Base::get();
 }
@@ -203 +223 @@
     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());
 }