It should be easier to reify lazy property names
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Nov 2017 19:08:37 +0000 (19:08 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Nov 2017 19:08:37 +0000 (19:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179734
<rdar://problem/35492521>

Reviewed by Keith Miller.

We reify lazy property names in a few different ways, each
specific to the JSCell implementation, in put() instead of having
a special function to do reification. Let's make that simpler.

This patch makes it easier to reify property names in a uniform
manner, and does so in JSFunction. As a follow up I'll use the
same mechanics for:

ClonedArguments   callee, iteratorSymbol (Symbol.iterator)
ErrorConstructor  stackTraceLimit
ErrorInstance     line, column, sourceURL, stack
GenericArguments  length, callee, iteratorSymbol (Symbol.iterator)
GetterSetter      RELEASE_ASSERT_NOT_REACHED()
JSArray           length
RegExpObject      lastIndex
StringObject      length

* runtime/ClassInfo.h: Add reifyPropertyNameIfNeeded to method table.
* runtime/JSCell.cpp:
(JSC::JSCell::reifyPropertyNameIfNeeded): by default, don't reify.
* runtime/JSCell.h:
* runtime/JSFunction.cpp: `name` and `length` can be reified.
(JSC::JSFunction::reifyPropertyNameIfNeeded):
(JSC::JSFunction::put):
(JSC::JSFunction::reifyLength):
(JSC::JSFunction::reifyName):
(JSC::JSFunction::reifyLazyPropertyIfNeeded):
(JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
(JSC::JSFunction::reifyLazyLengthIfNeeded):
(JSC::JSFunction::reifyLazyNameIfNeeded):
(JSC::JSFunction::reifyLazyBoundNameIfNeeded):
* runtime/JSFunction.h:
(JSC::JSFunction::isLazy):
(JSC::JSFunction::isReified):
* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal): do the reification here.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ClassInfo.h
Source/JavaScriptCore/runtime/JSCell.cpp
Source/JavaScriptCore/runtime/JSCell.h
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/JSFunction.h
Source/JavaScriptCore/runtime/JSObjectInlines.h

index 92890c0..92e8177 100644 (file)
@@ -1,3 +1,48 @@
+2017-11-16  JF Bastien  <jfbastien@apple.com>
+
+        It should be easier to reify lazy property names
+        https://bugs.webkit.org/show_bug.cgi?id=179734
+        <rdar://problem/35492521>
+
+        Reviewed by Keith Miller.
+
+        We reify lazy property names in a few different ways, each
+        specific to the JSCell implementation, in put() instead of having
+        a special function to do reification. Let's make that simpler.
+
+        This patch makes it easier to reify property names in a uniform
+        manner, and does so in JSFunction. As a follow up I'll use the
+        same mechanics for:
+
+        ClonedArguments   callee, iteratorSymbol (Symbol.iterator)
+        ErrorConstructor  stackTraceLimit
+        ErrorInstance     line, column, sourceURL, stack
+        GenericArguments  length, callee, iteratorSymbol (Symbol.iterator)
+        GetterSetter      RELEASE_ASSERT_NOT_REACHED()
+        JSArray           length
+        RegExpObject      lastIndex
+        StringObject      length
+
+        * runtime/ClassInfo.h: Add reifyPropertyNameIfNeeded to method table.
+        * runtime/JSCell.cpp:
+        (JSC::JSCell::reifyPropertyNameIfNeeded): by default, don't reify.
+        * runtime/JSCell.h:
+        * runtime/JSFunction.cpp: `name` and `length` can be reified.
+        (JSC::JSFunction::reifyPropertyNameIfNeeded):
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::reifyLength):
+        (JSC::JSFunction::reifyName):
+        (JSC::JSFunction::reifyLazyPropertyIfNeeded):
+        (JSC::JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded):
+        (JSC::JSFunction::reifyLazyLengthIfNeeded):
+        (JSC::JSFunction::reifyLazyNameIfNeeded):
+        (JSC::JSFunction::reifyLazyBoundNameIfNeeded):
+        * runtime/JSFunction.h:
+        (JSC::JSFunction::isLazy):
+        (JSC::JSFunction::isReified):
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::putDirectInternal): do the reification here.
+
 2017-11-16  Robin Morisset  <rmorisset@apple.com>
 
         Provide a runtime option for disabling the optimization of recursive tail calls
index 96e7c60..925d548 100644 (file)
@@ -43,7 +43,7 @@ struct MethodTable {
 
     typedef void (*VisitChildrenFunctionPtr)(JSCell*, SlotVisitor&);
     VisitChildrenFunctionPtr visitChildren;
-    
+
     typedef CallType (*GetCallDataFunctionPtr)(JSCell*, CallData&);
     GetCallDataFunctionPtr getCallData;
 
@@ -103,7 +103,7 @@ struct MethodTable {
 
     typedef ArrayBuffer* (*SlowDownAndWasteMemory)(JSArrayBufferView*);
     SlowDownAndWasteMemory slowDownAndWasteMemory;
-    
+
     typedef RefPtr<ArrayBufferView> (*GetTypedArrayImpl)(JSArrayBufferView*);
     GetTypedArrayImpl getTypedArrayImpl;
 
@@ -127,9 +127,12 @@ struct MethodTable {
 
     typedef size_t (*EstimatedSizeFunctionPtr)(JSCell*);
     EstimatedSizeFunctionPtr estimatedSize;
-    
+
     typedef void (*VisitOutputConstraintsPtr)(JSCell*, SlotVisitor&);
     VisitOutputConstraintsPtr visitOutputConstraints;
+
+    using ReifyPropertyNameIfNeededPtr = PropertyReificationResult (*)(JSCell*, ExecState*, PropertyName&);
+    ReifyPropertyNameIfNeededPtr reifyPropertyNameIfNeeded;
 };
 
 #define CREATE_MEMBER_CHECKER(member) \
@@ -183,7 +186,8 @@ struct MethodTable {
         &ClassName::dumpToStream, \
         &ClassName::heapSnapshot, \
         &ClassName::estimatedSize, \
-        &ClassName::visitOutputConstraints \
+        &ClassName::visitOutputConstraints, \
+        &ClassName::reifyPropertyNameIfNeeded, \
     }, \
     ClassName::TypedArrayStorageType
 
index 44da987..14346e8 100644 (file)
@@ -63,6 +63,11 @@ size_t JSCell::estimatedSize(JSCell* cell)
     return cell->cellSize();
 }
 
+PropertyReificationResult JSCell::reifyPropertyNameIfNeeded(JSCell*, ExecState*, PropertyName&)
+{
+    return PropertyReificationResult::Nothing;
+}
+
 void JSCell::heapSnapshot(JSCell*, HeapSnapshotBuilder&)
 {
 }
index 46f4137..9fc0c9b 100644 (file)
@@ -46,6 +46,7 @@ class JSDestructibleObject;
 class JSGlobalObject;
 class LLIntOffsetsExtractor;
 class PropertyDescriptor;
+class PropertyName;
 class PropertyNameArray;
 class Structure;
 
@@ -59,6 +60,12 @@ enum class GCDeferralContextArgPresense {
     DoesNotHaveArg
 };
 
+enum class PropertyReificationResult {
+    Nothing,
+    Something,
+    TriedButFailed, // Sometimes the property name already exists but has special behavior and can't be reified, e.g. Array.length.
+};
+
 template<typename T> void* allocateCell(Heap&, size_t = sizeof(T));
 template<typename T> void* tryAllocateCell(Heap&, size_t = sizeof(T));
 template<typename T> void* allocateCell(Heap&, GCDeferralContext*, size_t = sizeof(T));
@@ -170,6 +177,8 @@ public:
     static void visitChildren(JSCell*, SlotVisitor&);
     static void visitOutputConstraints(JSCell*, SlotVisitor&);
 
+    JS_EXPORT_PRIVATE static PropertyReificationResult reifyPropertyNameIfNeeded(JSCell*, ExecState*, PropertyName&);
+
     JS_EXPORT_PRIVATE static void heapSnapshot(JSCell*, HeapSnapshotBuilder&);
 
     // Object operations, with the toObject operation included.
index f863d0e..3686ff3 100644 (file)
@@ -216,6 +216,13 @@ void JSFunction::visitChildren(JSCell* cell, SlotVisitor& visitor)
     visitor.append(thisObject->m_rareData);
 }
 
+PropertyReificationResult JSFunction::reifyPropertyNameIfNeeded(JSCell* cell, ExecState* exec, PropertyName& propertyName)
+{
+    JSFunction* thisObject = jsCast<JSFunction*>(cell);
+    PropertyStatus propertyType = thisObject->reifyLazyPropertyIfNeeded(exec->vm(), exec, propertyName);
+    return isReified(propertyType) ? PropertyReificationResult::Something : PropertyReificationResult::Nothing;
+}
+
 CallType JSFunction::getCallData(JSCell* cell, CallData& callData)
 {
     JSFunction* thisObject = jsCast<JSFunction*>(cell);
@@ -442,8 +449,8 @@ bool JSFunction::put(JSCell* cell, ExecState* exec, PropertyName propertyName, J
     }
 
     if (thisObject->isHostOrBuiltinFunction()) {
-        LazyPropertyType propType = thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
-        if (propType == LazyPropertyType::IsLazyProperty)
+        PropertyStatus propertyType = thisObject->reifyLazyPropertyForHostOrBuiltinIfNeeded(vm, exec, propertyName);
+        if (isLazy(propertyType))
             slot.disableCaching();
         scope.release();
         return Base::put(thisObject, exec, propertyName, value, slot);
@@ -475,8 +482,8 @@ bool JSFunction::put(JSCell* cell, ExecState* exec, PropertyName propertyName, J
         }
         return typeError(exec, scope, slot.isStrictMode(), ASCIILiteral(ReadonlyPropertyWriteError));
     }
-    LazyPropertyType propType = thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);
-    if (propType == LazyPropertyType::IsLazyProperty)
+    PropertyStatus propertyType = thisObject->reifyLazyPropertyIfNeeded(vm, exec, propertyName);
+    if (isLazy(propertyType))
         slot.disableCaching();
     scope.release();
     return Base::put(thisObject, exec, propertyName, value, slot);
@@ -642,9 +649,8 @@ void JSFunction::reifyLength(VM& vm)
     JSValue initialValue = jsNumber(jsExecutable()->parameterCount());
     unsigned initialAttributes = PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly;
     const Identifier& identifier = vm.propertyNames->length;
-    putDirect(vm, identifier, initialValue, initialAttributes);
-
     rareData->setHasReifiedLength();
+    putDirect(vm, identifier, initialValue, initialAttributes);
 }
 
 void JSFunction::reifyName(VM& vm, ExecState* exec)
@@ -683,50 +689,66 @@ void JSFunction::reifyName(VM& vm, ExecState* exec, String name)
     else if (jsExecutable()->isSetter())
         name = makeString("set ", name);
 
-    putDirect(vm, propID, jsString(exec, name), initialAttributes);
     rareData->setHasReifiedName();
+    putDirect(vm, propID, jsString(exec, name), initialAttributes);
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+JSFunction::PropertyStatus JSFunction::reifyLazyPropertyIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
-    if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
-        return LazyPropertyType::IsLazyProperty;
-    if (propertyName == vm.propertyNames->name) {
-        if (!hasReifiedName())
-            reifyName(vm, exec);
-        return LazyPropertyType::IsLazyProperty;
-    }
-    return LazyPropertyType::NotLazyProperty;
+    if (isHostOrBuiltinFunction())
+        return PropertyStatus::Eager;
+    PropertyStatus lazyLength = reifyLazyLengthIfNeeded(vm, exec, propertyName);
+    if (isLazy(lazyLength))
+        return lazyLength;
+    PropertyStatus lazyName = reifyLazyNameIfNeeded(vm, exec, propertyName);
+    if (isLazy(lazyName))
+        return lazyName;
+    return PropertyStatus::Eager;
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+JSFunction::PropertyStatus JSFunction::reifyLazyPropertyForHostOrBuiltinIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
     ASSERT(isHostOrBuiltinFunction());
     if (isBuiltinFunction()) {
-        if (reifyLazyLengthIfNeeded(vm, exec, propertyName) == LazyPropertyType::IsLazyProperty)
-            return LazyPropertyType::IsLazyProperty;
+        PropertyStatus lazyLength = reifyLazyLengthIfNeeded(vm, exec, propertyName);
+        if (isLazy(lazyLength))
+            return lazyLength;
     }
     return reifyLazyBoundNameIfNeeded(vm, exec, propertyName);
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyLazyLengthIfNeeded(VM& vm, ExecState*, PropertyName propertyName)
+JSFunction::PropertyStatus JSFunction::reifyLazyLengthIfNeeded(VM& vm, ExecState*, PropertyName propertyName)
 {
     if (propertyName == vm.propertyNames->length) {
-        if (!hasReifiedLength())
+        if (!hasReifiedLength()) {
             reifyLength(vm);
-        return LazyPropertyType::IsLazyProperty;
+            return PropertyStatus::Reified;
+        }
+        return PropertyStatus::Lazy;
     }
-    return LazyPropertyType::NotLazyProperty;
+    return PropertyStatus::Eager;
 }
 
-JSFunction::LazyPropertyType JSFunction::reifyLazyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+JSFunction::PropertyStatus JSFunction::reifyLazyNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
+{
+    if (propertyName == vm.propertyNames->name) {
+        if (!hasReifiedName()) {
+            reifyName(vm, exec);
+            return PropertyStatus::Reified;
+        }
+        return PropertyStatus::Lazy;
+    }
+    return PropertyStatus::Eager;
+}
+
+JSFunction::PropertyStatus JSFunction::reifyLazyBoundNameIfNeeded(VM& vm, ExecState* exec, PropertyName propertyName)
 {
     const Identifier& nameIdent = vm.propertyNames->name;
     if (propertyName != nameIdent)
-        return LazyPropertyType::NotLazyProperty;
+        return PropertyStatus::Eager;
 
     if (hasReifiedName())
-        return LazyPropertyType::IsLazyProperty;
+        return PropertyStatus::Lazy;
 
     if (isBuiltinFunction())
         reifyName(vm, exec);
@@ -734,10 +756,10 @@ JSFunction::LazyPropertyType JSFunction::reifyLazyBoundNameIfNeeded(VM& vm, Exec
         FunctionRareData* rareData = this->rareData(vm);
         String name = makeString("bound ", static_cast<NativeExecutable*>(m_executable.get())->name());
         unsigned initialAttributes = PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly;
-        putDirect(vm, nameIdent, jsString(exec, name), initialAttributes);
         rareData->setHasReifiedName();
+        putDirect(vm, nameIdent, jsString(exec, name), initialAttributes);
     }
-    return LazyPropertyType::IsLazyProperty;
+    return PropertyStatus::Reified;
 }
 
 } // namespace JSC
index 4794502..041ef31 100644 (file)
@@ -172,6 +172,7 @@ protected:
 
     static void visitChildren(JSCell*, SlotVisitor&);
 
+    static PropertyReificationResult reifyPropertyNameIfNeeded(JSCell*, ExecState*, PropertyName&);
 
 private:
     static JSFunction* createImpl(VM& vm, FunctionExecutable* executable, JSScope* scope, Structure* structure)
@@ -188,11 +189,19 @@ private:
     void reifyName(VM&, ExecState*);
     void reifyName(VM&, ExecState*, String name);
 
-    enum class LazyPropertyType { NotLazyProperty, IsLazyProperty };
-    LazyPropertyType reifyLazyPropertyIfNeeded(VM&, ExecState*, PropertyName);
-    LazyPropertyType reifyLazyPropertyForHostOrBuiltinIfNeeded(VM&, ExecState*, PropertyName);
-    LazyPropertyType reifyLazyLengthIfNeeded(VM&, ExecState*, PropertyName);
-    LazyPropertyType reifyLazyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
+    enum class PropertyStatus {
+        Eager,
+        Lazy,
+        Reified,
+    };
+    static bool isLazy(PropertyStatus property) { return property == PropertyStatus::Lazy || property == PropertyStatus::Reified; }
+    static bool isReified(PropertyStatus property) { return property == PropertyStatus::Reified; }
+
+    PropertyStatus reifyLazyPropertyIfNeeded(VM&, ExecState*, PropertyName);
+    PropertyStatus reifyLazyPropertyForHostOrBuiltinIfNeeded(VM&, ExecState*, PropertyName);
+    PropertyStatus reifyLazyLengthIfNeeded(VM&, ExecState*, PropertyName);
+    PropertyStatus reifyLazyNameIfNeeded(VM&, ExecState*, PropertyName);
+    PropertyStatus reifyLazyBoundNameIfNeeded(VM&, ExecState*, PropertyName);
 
     friend class LLIntOffsetsExtractor;
 
index 89811c9..29cb941 100644 (file)
@@ -260,6 +260,12 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
     ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this));
     ASSERT(!parseIndex(propertyName));
 
+    switch (methodTable(vm)->reifyPropertyNameIfNeeded(this, globalObject(vm)->globalExec(), propertyName)) {
+    case PropertyReificationResult::Nothing: break;
+    case PropertyReificationResult::Something: break;
+    case PropertyReificationResult::TriedButFailed: RELEASE_ASSERT_NOT_REACHED();
+    }
+
     StructureID structureID = this->structureID();
     Structure* structure = vm.heap.structureIDTable().get(structureID);
     if (structure->isDictionary()) {