Title: [128265] trunk/Source/_javascript_Core
Revision
128265
Author
gga...@apple.com
Date
2012-09-11 23:14:56 -0700 (Tue, 11 Sep 2012)

Log Message

Don't allocate a backing store just for a function's name
https://bugs.webkit.org/show_bug.cgi?id=96468

Reviewed by Oliver Hunt.

Treat function.name like function.length etc., and use a custom getter.
This saves space in closures.

* debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::functionName):
* debugger/DebuggerCallFrame.h:
(DebuggerCallFrame): Updated for interface change.

* runtime/Executable.h:
(JSC::JSFunction::JSFunction): Do a little inlining.

* runtime/JSFunction.cpp:
(JSC::JSFunction::finishCreation): Gone now. That's the point of the patch.

(JSC::JSFunction::name):
(JSC::JSFunction::displayName):
(JSC::JSFunction::nameGetter):
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::getOwnPropertyDescriptor):
(JSC::JSFunction::getOwnPropertyNames):
(JSC::JSFunction::put):
(JSC::JSFunction::deleteProperty):
(JSC::JSFunction::defineOwnProperty): Added custom accessors for .name
just like .length and others.

* runtime/JSFunction.h:
(JSC::JSFunction::create):
(JSFunction): Updated for interface changes.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (128264 => 128265)


--- trunk/Source/_javascript_Core/ChangeLog	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-09-12 06:14:56 UTC (rev 128265)
@@ -1,3 +1,39 @@
+2012-09-11  Geoffrey Garen  <gga...@apple.com>
+
+        Don't allocate a backing store just for a function's name
+        https://bugs.webkit.org/show_bug.cgi?id=96468
+
+        Reviewed by Oliver Hunt.
+
+        Treat function.name like function.length etc., and use a custom getter.
+        This saves space in closures.
+
+        * debugger/DebuggerCallFrame.cpp:
+        (JSC::DebuggerCallFrame::functionName):
+        * debugger/DebuggerCallFrame.h:
+        (DebuggerCallFrame): Updated for interface change.
+
+        * runtime/Executable.h:
+        (JSC::JSFunction::JSFunction): Do a little inlining.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::finishCreation): Gone now. That's the point of the patch.
+
+        (JSC::JSFunction::name):
+        (JSC::JSFunction::displayName):
+        (JSC::JSFunction::nameGetter):
+        (JSC::JSFunction::getOwnPropertySlot):
+        (JSC::JSFunction::getOwnPropertyDescriptor):
+        (JSC::JSFunction::getOwnPropertyNames):
+        (JSC::JSFunction::put):
+        (JSC::JSFunction::deleteProperty):
+        (JSC::JSFunction::defineOwnProperty): Added custom accessors for .name
+        just like .length and others.
+
+        * runtime/JSFunction.h:
+        (JSC::JSFunction::create):
+        (JSFunction): Updated for interface changes.
+
 2012-09-11  Mark Hahnenberg  <mhahnenb...@apple.com>
 
         IncrementalSweeper should not sweep/free Zapped blocks

Modified: trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp (128264 => 128265)


--- trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.cpp	2012-09-12 06:14:56 UTC (rev 128265)
@@ -36,18 +36,18 @@
 
 namespace JSC {
 
-const String* DebuggerCallFrame::functionName() const
+String DebuggerCallFrame::functionName() const
 {
     if (!m_callFrame->codeBlock())
-        return 0;
+        return String();
 
     if (!m_callFrame->callee())
-        return 0;
+        return String();
 
     JSObject* function = m_callFrame->callee();
     if (!function || !function->inherits(&JSFunction::s_info))
-        return 0;
-    return &jsCast<JSFunction*>(function)->name(m_callFrame);
+        return String();
+    return jsCast<JSFunction*>(function)->name(m_callFrame);
 }
     
 String DebuggerCallFrame::calculatedFunctionName() const

Modified: trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.h (128264 => 128265)


--- trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.h	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/debugger/DebuggerCallFrame.h	2012-09-12 06:14:56 UTC (rev 128265)
@@ -51,7 +51,7 @@
         CallFrame* callFrame() const { return m_callFrame; }
         JSGlobalObject* dynamicGlobalObject() const { return m_callFrame->dynamicGlobalObject(); }
         JSScope* scope() const { return m_callFrame->scope(); }
-        JS_EXPORT_PRIVATE const String* functionName() const;
+        JS_EXPORT_PRIVATE String functionName() const;
         JS_EXPORT_PRIVATE String calculatedFunctionName() const;
         JS_EXPORT_PRIVATE Type type() const;
         JS_EXPORT_PRIVATE JSObject* thisObject() const;

Modified: trunk/Source/_javascript_Core/runtime/Executable.h (128264 => 128265)


--- trunk/Source/_javascript_Core/runtime/Executable.h	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/runtime/Executable.h	2012-09-12 06:14:56 UTC (rev 128265)
@@ -31,6 +31,7 @@
 #include "HandlerInfo.h"
 #include "JSFunction.h"
 #include "Interpreter.h"
+#include "JSGlobalObject.h"
 #include "LLIntCLoop.h"
 #include "Nodes.h"
 #include "SamplingTool.h"
@@ -754,6 +755,13 @@
         WriteBarrier<SharedSymbolTable> m_symbolTable;
     };
 
+    inline JSFunction::JSFunction(JSGlobalData& globalData, FunctionExecutable* executable, JSScope* scope)
+        : Base(globalData, scope->globalObject()->functionStructure())
+        , m_executable(globalData, this, executable)
+        , m_scope(globalData, this, scope)
+    {
+    }
+
     inline FunctionExecutable* JSFunction::jsExecutable() const
     {
         ASSERT(!isHostFunctionNonInline());

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (128264 => 128265)


--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp	2012-09-12 06:14:56 UTC (rev 128265)
@@ -84,13 +84,6 @@
 {
 }
 
-JSFunction::JSFunction(ExecState* exec, FunctionExecutable* executable, JSScope* scope)
-    : Base(exec->globalData(), scope->globalObject()->functionStructure())
-    , m_executable(exec->globalData(), this, executable)
-    , m_scope(exec->globalData(), this, scope)
-{
-}
-
 void JSFunction::finishCreation(ExecState* exec, NativeExecutable* executable, int length, const String& name)
 {
     Base::finishCreation(exec->globalData());
@@ -100,20 +93,6 @@
     putDirect(exec->globalData(), exec->propertyNames().length, jsNumber(length), DontDelete | ReadOnly | DontEnum);
 }
 
-void JSFunction::finishCreation(ExecState* exec, FunctionExecutable* executable, JSScope* scope)
-{
-    JSGlobalData& globalData = exec->globalData();
-    Base::finishCreation(globalData);
-    ASSERT(inherits(&s_info));
-
-    // Switching the structure here is only safe if we currently have the function structure!
-    ASSERT(structure() == scope->globalObject()->functionStructure());
-    setStructureAndReallocateStorageIfNecessary(
-        globalData,
-        scope->globalObject()->namedFunctionStructure());
-    putDirectOffset(globalData, scope->globalObject()->functionNameOffset(), executable->nameValue());
-}
-
 Structure* JSFunction::cacheInheritorID(ExecState* exec)
 {
     JSValue prototype = get(exec, exec->globalData().propertyNames->prototype);
@@ -124,12 +103,12 @@
     return m_cachedInheritorID.get();
 }
 
-const String& JSFunction::name(ExecState* exec)
+String JSFunction::name(ExecState* exec)
 {
-    return asString(getDirect(exec->globalData(), exec->globalData().propertyNames->name))->tryGetValue();
+    return get(exec, exec->globalData().propertyNames->name).toWTFString(exec);
 }
 
-const String JSFunction::displayName(ExecState* exec)
+String JSFunction::displayName(ExecState* exec)
 {
     JSValue displayName = getDirect(exec->globalData(), exec->globalData().propertyNames->displayName);
     
@@ -213,6 +192,13 @@
     return jsNumber(thisObj->jsExecutable()->parameterCount());
 }
 
+JSValue JSFunction::nameGetter(ExecState*, JSValue slotBase, PropertyName)
+{
+    JSFunction* thisObj = jsCast<JSFunction*>(slotBase);
+    ASSERT(!thisObj->isHostFunction());
+    return thisObj->jsExecutable()->nameValue();
+}
+
 bool JSFunction::getOwnPropertySlot(JSCell* cell, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     JSFunction* thisObject = jsCast<JSFunction*>(cell);
@@ -251,6 +237,11 @@
         return true;
     }
 
+    if (propertyName == exec->propertyNames().name) {
+        slot.setCacheableCustom(thisObject, nameGetter);
+        return true;
+    }
+
     if (propertyName == exec->propertyNames().caller) {
         if (thisObject->jsExecutable()->isStrictMode()) {
             bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
@@ -299,6 +290,11 @@
         return true;
     }
     
+    if (propertyName == exec->propertyNames().name) {
+        descriptor.setDescriptor(thisObject->jsExecutable()->nameValue(), ReadOnly | DontEnum | DontDelete);
+        return true;
+    }
+
     if (propertyName == exec->propertyNames().caller) {
         if (thisObject->jsExecutable()->isStrictMode()) {
             bool result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
@@ -327,6 +323,7 @@
         propertyNames.add(exec->propertyNames().arguments);
         propertyNames.add(exec->propertyNames().caller);
         propertyNames.add(exec->propertyNames().length);
+        propertyNames.add(exec->propertyNames().name);
     }
     Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);
 }
@@ -356,7 +353,7 @@
         Base::put(thisObject, exec, propertyName, value, slot);
         return;
     }
-    if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().length || propertyName == exec->propertyNames().caller) {
+    if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().length || propertyName == exec->propertyNames().name || propertyName == exec->propertyNames().caller) {
         if (slot.isStrictMode())
             throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
         return;
@@ -371,6 +368,7 @@
     if (!thisObject->isHostFunction() && !exec->globalData().isInDefineOwnProperty()
         && (propertyName == exec->propertyNames().arguments
             || propertyName == exec->propertyNames().length
+            || propertyName == exec->propertyNames().name
             || propertyName == exec->propertyNames().prototype
             || propertyName == exec->propertyNames().caller))
         return false;
@@ -409,6 +407,8 @@
         valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), exec->interpreter()->retrieveCallerFromVMCode(exec, thisObject));
     } else if (propertyName == exec->propertyNames().length)
         valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), jsNumber(thisObject->jsExecutable()->parameterCount()));
+    else if (propertyName == exec->propertyNames().name)
+        valueCheck = !descriptor.value() || sameValue(exec, descriptor.value(), thisObject->jsExecutable()->nameValue());
     else
         return Base::defineOwnProperty(object, exec, propertyName, descriptor, throwException);
      

Modified: trunk/Source/_javascript_Core/runtime/JSFunction.h (128264 => 128265)


--- trunk/Source/_javascript_Core/runtime/JSFunction.h	2012-09-12 05:52:21 UTC (rev 128264)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.h	2012-09-12 06:14:56 UTC (rev 128265)
@@ -59,14 +59,15 @@
 
         static JSFunction* create(ExecState* exec, FunctionExecutable* executable, JSScope* scope)
         {
-            JSFunction* function = new (NotNull, allocateCell<JSFunction>(*exec->heap())) JSFunction(exec, executable, scope);
+            JSGlobalData& globalData = exec->globalData();
+            JSFunction* function = new (NotNull, allocateCell<JSFunction>(globalData.heap)) JSFunction(globalData, executable, scope);
             ASSERT(function->structure()->globalObject());
-            function->finishCreation(exec, executable, scope);
+            function->finishCreation(globalData);
             return function;
         }
         
-        JS_EXPORT_PRIVATE const String& name(ExecState*);
-        JS_EXPORT_PRIVATE const String displayName(ExecState*);
+        JS_EXPORT_PRIVATE String name(ExecState*);
+        JS_EXPORT_PRIVATE String displayName(ExecState*);
         const String calculatedDisplayName(ExecState*);
 
         JSScope* scope()
@@ -137,10 +138,10 @@
         const static unsigned StructureFlags = OverridesGetOwnPropertySlot | ImplementsHasInstance | OverridesVisitChildren | OverridesGetPropertyNames | JSObject::StructureFlags;
 
         JS_EXPORT_PRIVATE JSFunction(ExecState*, JSGlobalObject*, Structure*);
-        JSFunction(ExecState*, FunctionExecutable*, JSScope*);
+        JSFunction(JSGlobalData&, FunctionExecutable*, JSScope*);
         
         void finishCreation(ExecState*, NativeExecutable*, int length, const String& name);
-        void finishCreation(ExecState*, FunctionExecutable*, JSScope*);
+        using Base::finishCreation;
 
         Structure* cacheInheritorID(ExecState*);
 
@@ -163,6 +164,7 @@
         static JSValue argumentsGetter(ExecState*, JSValue, PropertyName);
         static JSValue callerGetter(ExecState*, JSValue, PropertyName);
         static JSValue lengthGetter(ExecState*, JSValue, PropertyName);
+        static JSValue nameGetter(ExecState*, JSValue, PropertyName);
 
         WriteBarrier<ExecutableBase> m_executable;
         WriteBarrier<JSScope> m_scope;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to