Changeset 64655 in webkit

Timestamp:

Aug 4, 2010 10:18:07 AM (14 years ago)

Author:

commit-queue@webkit.org

Message:

2010-08-04 Nathan Lawrence <​nlawrence@apple.com>

Reviewed by Darin Adler.

Refactoring MarkStack::append to take a reference. This is in
preparation for movable objects when we will need to update pointers.
​http://bugs.webkit.org/show_bug.cgi?id=41177

Unless otherwise noted, all changes are to either return by reference
or pass a reference to MarkStack::append.

• bytecode/CodeBlock.cpp: (JSC::CodeBlock::markAggregate):
• runtime/Collector.cpp: (JSC::Heap::markConservatively):

Added a temporary variable to prevent marking from changing an
unknown value on the stack

• runtime/JSCell.h: (JSC::JSValue::asCell): (JSC::MarkStack::append): (JSC::MarkStack::appendInternal):
• runtime/JSGlobalObject.cpp: (JSC::markIfNeeded):
• runtime/JSONObject.cpp: (JSC::Stringifier::Holder::object):
• runtime/JSObject.h: (JSC::JSObject::prototype):
• runtime/JSStaticScopeObject.cpp: (JSC::JSStaticScopeObject::markChildren):
• runtime/JSValue.h: (JSC::JSValue::JSValue): (JSC::JSValue::asCell):
• runtime/MarkStack.h:
• runtime/NativeErrorConstructor.cpp: (JSC::NativeErrorConstructor::createStructure):

Changed the structure flags to include a custom markChildren.

(JSC::NativeErrorConstructor::markChildren):

Update the prototype of the stored structure.

• runtime/NativeErrorConstructor.h:

Added structure flags.

• runtime/Structure.h: (JSC::Structure::storedPrototype):

2010-08-04 Nathan Lawrence <​nlawrence@apple.com>

Reviewed by Darin Adler.

Removed unneeded marking. We need to remove this marking in order to have
MarkStack::append take references for updating movable objects.

​https://bugs.webkit.org/show_bug.cgi?id=41177

• JSValueWrapper.cpp: (JSValueWrapper::JSObjectMark):

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/bytecode/CodeBlock.cpp (modified) (1 diff)
• JavaScriptCore/runtime/Collector.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSCell.h (modified) (4 diffs)
• JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/JSONObject.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSObject.h (modified) (2 diffs)
• JavaScriptCore/runtime/JSStaticScopeObject.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSValue.h (modified) (6 diffs)
• JavaScriptCore/runtime/MarkStack.h (modified) (2 diffs)
• JavaScriptCore/runtime/NativeErrorConstructor.cpp (modified) (1 diff)
• JavaScriptCore/runtime/NativeErrorConstructor.h (modified) (1 diff)
• JavaScriptCore/runtime/Structure.h (modified) (1 diff)
• JavaScriptGlue/ChangeLog (modified) (1 diff)
• JavaScriptGlue/JSValueWrapper.cpp (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-08-04  Nathan Lawrence  <nlawrence@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Refactoring MarkStack::append to take a reference.  This is in
+        preparation for movable objects when we will need to update pointers.
+        http://bugs.webkit.org/show_bug.cgi?id=41177
+
+        Unless otherwise noted, all changes are to either return by reference
+        or pass a reference to MarkStack::append.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::markAggregate):
+        * runtime/Collector.cpp:
+        (JSC::Heap::markConservatively):
+            Added a temporary variable to prevent marking from changing an
+            unknown value on the stack
+        * runtime/JSCell.h:
+        (JSC::JSValue::asCell):
+        (JSC::MarkStack::append):
+        (JSC::MarkStack::appendInternal):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::markIfNeeded):
+        * runtime/JSONObject.cpp:
+        (JSC::Stringifier::Holder::object):
+        * runtime/JSObject.h:
+        (JSC::JSObject::prototype):
+        * runtime/JSStaticScopeObject.cpp:
+        (JSC::JSStaticScopeObject::markChildren):
+        * runtime/JSValue.h:
+        (JSC::JSValue::JSValue):
+        (JSC::JSValue::asCell):
+        * runtime/MarkStack.h:
+        * runtime/NativeErrorConstructor.cpp:
+        (JSC::NativeErrorConstructor::createStructure):
+            Changed the structure flags to include a custom markChildren.
+        (JSC::NativeErrorConstructor::markChildren):
+            Update the prototype of the stored structure.
+        * runtime/NativeErrorConstructor.h:
+            Added structure flags.
+        * runtime/Structure.h:
+        (JSC::Structure::storedPrototype):
+
 2010-08-03  Nathan Lawrence  <nlawrence@apple.com>
 

trunk/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1514 +1514 @@
 {
     for (size_t i = 0; i < m_constantRegisters.size(); ++i)
-        markStack.append(m_constantRegisters[i].jsValue());
+        markStack.append(m_constantRegisters[i]);
     for (size_t i = 0; i < m_functionExprs.size(); ++i)
         m_functionExprs[i]->markAggregate(markStack);

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -692 +692 @@
                 if (m_heap.collectorBlock(block) != blockAddr)
                     continue;
-                markStack.append(reinterpret_cast<JSCell*>(xAsBits));
+                // While markStack.append normally takes a reference to update,
+                // we don't actually want the heap to be updated since we don't
+                // know for sure that it's actually a pointer. In the future
+                // this will be replaced by some appendRoot function for this
+                // specific case.
+                JSCell* cell = reinterpret_cast<JSCell*>(xAsBits);
+                markStack.append(cell);
                 markStack.drain();
             }

trunk/JavaScriptCore/runtime/JSCell.h

@@ -36 +36 @@
 
     class JSCell : public NoncopyableCustomAllocated {
+        friend class CollectorHeap;
         friend class GetterSetter;
         friend class Heap;
@@ -238 +239 @@
 
 #if !USE(JSVALUE32_64)
-    ALWAYS_INLINE JSCell* JSValue::asCell() const
+    ALWAYS_INLINE JSCell*& JSValue::asCell()
+    {
+        ASSERT(isCell());
+        return m_ptr;
+    }
+
+    ALWAYS_INLINE JSCell* const& JSValue::asCell() const
     {
         ASSERT(isCell());
@@ -330 +337 @@
     }
 
-    ALWAYS_INLINE void MarkStack::append(JSCell* cell)
+    template<typename T>
+    ALWAYS_INLINE void MarkStack::append(T*& cell)
+    {
+        // References in C++ are not covariant.  JSObject* being a subtype of JSCell*
+        // does not mean that JSObject*& can be used as a subtype of JSCell*& because
+        // treating a JSObject*& as a JSCell*& would allow us to change the pointer to
+        // point to something that is a JSCell but not a JSObject.
+        //
+        // In this case, we need to be able to change the pointer, and although we know
+        // it to be safe, C++ doesn't, requiring us to use templated functions that
+        // pass a casted version to an internal function.
+        //
+        // Currently we're not doing anything with the value of the pointer, so nothing
+        // unsafe will happen.  In the future, when we have movable objects, we will be
+        // changing the value of the pointer to be the new location, in which case the
+        // type will be preserved.
+        JSCell*& ptr = *reinterpret_cast<JSCell**>(&cell);
+        appendInternal(ptr);
+    }
+
+    ALWAYS_INLINE void MarkStack::append(JSValue& value)
+    {
+        ASSERT(value);
+        if (value.isCell())
+            appendInternal(value.asCell());
+    }
+
+    ALWAYS_INLINE void MarkStack::append(Register& reg)
+    {
+        JSValue value = reg.jsValue();
+        append(value);
+        reg = value;
+    }
+
+    inline void MarkStack::appendInternal(JSCell*& cell)
     {
         ASSERT(!m_isCheckingForDefaultMarkViolation);
@@ -341 +382 @@
     }
 
-    ALWAYS_INLINE void MarkStack::append(JSValue value)
-    {
-        ASSERT(value);
-        if (value.isCell())
-            append(value.asCell());
-    }
-
     inline Heap* Heap::heap(JSValue v)
     {

trunk/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -81 +81 @@
 static const int preferredScriptCheckTimeInterval = 1000;
 
-static inline void markIfNeeded(MarkStack& markStack, JSValue v)
+template<typename T> static inline void markIfNeeded(MarkStack& markStack, T*& v)
 {
     if (v)
@@ -87 +87 @@
 }
 
-static inline void markIfNeeded(MarkStack& markStack, const RefPtr<Structure>& s)
+static inline void markIfNeeded(MarkStack& markStack, RefPtr<Structure>& s)
 {
     if (s)
-        markIfNeeded(markStack, s->storedPrototype());
+        markStack.append(s->storedPrototype());
 }
 

trunk/JavaScriptCore/runtime/JSONObject.cpp

@@ -83 +83 @@
         Holder(JSObject*);
 
-        JSObject* object() const { return m_object; }
+        JSObject*& object() { return m_object; }
+        JSObject* const& object() const { return m_object; }
 
         bool appendNextProperty(Stringifier&, StringBuilder&);
 
     private:
-        JSObject* const m_object;
+        JSObject* m_object;
         const bool m_isArray;
         bool m_isJSArray;

trunk/JavaScriptCore/runtime/JSObject.h

@@ -87 +87 @@
         virtual ~JSObject();
 
-        JSValue prototype() const;
+        JSValue& prototype();
+        const JSValue& prototype() const;
         void setPrototype(JSValue prototype);
         bool setPrototypeWithCycleCheck(JSValue prototype);
@@ -313 +314 @@
 }
 
-inline JSValue JSObject::prototype() const
+inline JSValue& JSObject::prototype()
+{
+    return m_structure->storedPrototype();
+}
+
+inline const JSValue& JSObject::prototype() const
 {
     return m_structure->storedPrototype();

trunk/JavaScriptCore/runtime/JSStaticScopeObject.cpp

@@ -35 +35 @@
 {
     JSVariableObject::markChildren(markStack);
-    markStack.append(d()->registerStore.jsValue());
+    markStack.append(d()->registerStore);
 }
 

trunk/JavaScriptCore/runtime/JSValue.h

@@ -200 +200 @@
 
         bool isCell() const;
-        JSCell* asCell() const;
+        JSCell*& asCell();
+        JSCell* const& asCell() const;
         bool isValidCallee();
 
@@ -241 +242 @@
                 int32_t payload;
             } asBits;
+            struct {
+                int32_t tag;
+                JSCell* ptr;
+            } asPtr;
 #else
             struct {
@@ -246 +251 @@
                 int32_t tag;
             } asBits;
+            struct {
+                JSCell* ptr;
+                int32_t tag;
+            } asPtr;
 #endif
         } u;
@@ -492 +501 @@
         else
             u.asBits.tag = EmptyValueTag;
-        u.asBits.payload = reinterpret_cast<int32_t>(ptr);
+        u.asPtr.ptr = ptr;
 #if ENABLE(JSC_ZOMBIES)
         ASSERT(!isZombie());
@@ -504 +513 @@
         else
             u.asBits.tag = EmptyValueTag;
-        u.asBits.payload = reinterpret_cast<int32_t>(const_cast<JSCell*>(ptr));
+        u.asPtr.ptr = const_cast<JSCell*>(ptr);
 #if ENABLE(JSC_ZOMBIES)
         ASSERT(!isZombie());
@@ -599 +608 @@
     }
     
-    ALWAYS_INLINE JSCell* JSValue::asCell() const
+    ALWAYS_INLINE JSCell* const& JSValue::asCell() const
     {
         ASSERT(isCell());
-        return reinterpret_cast<JSCell*>(u.asBits.payload);
+        return u.asPtr.ptr;
+    }
+
+    ALWAYS_INLINE JSCell*& JSValue::asCell()
+    {
+        ASSERT(isCell());
+        return u.asPtr.ptr;
     }
 

trunk/JavaScriptCore/runtime/MarkStack.h

@@ -47 +47 @@
         }
 
-        ALWAYS_INLINE void append(JSValue);
-        void append(JSCell*);
+        template<typename T> ALWAYS_INLINE void append(T*& cell);
+        ALWAYS_INLINE void append(JSValue&);
+        ALWAYS_INLINE void append(Register&);
         
         ALWAYS_INLINE void appendValues(Register* values, size_t count, MarkSetProperties properties = NoNullValues)
@@ -71 +72 @@
 
     private:
+        void appendInternal(JSCell*&);
+
         void markChildren(JSCell*);
 

trunk/JavaScriptCore/runtime/NativeErrorConstructor.cpp

@@ -69 +69 @@
 }
 
+PassRefPtr<Structure> NativeErrorConstructor::createStructure(JSObject* prototype)
+{
+    return Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount);
+}
+
+void NativeErrorConstructor::markChildren(MarkStack& markStack)
+{
+    InternalFunction::markChildren(markStack);
+
+    markStack.append(m_errorStructure->storedPrototype());
+}
+
 } // namespace JSC

trunk/JavaScriptCore/runtime/NativeErrorConstructor.h

@@ -37 +37 @@
 
         Structure* errorStructure() { return m_errorStructure.get(); }
+        static PassRefPtr<Structure> createStructure(JSObject* prototype);
+
+    protected:
+        static const unsigned StructureFlags = InternalFunction::StructureFlags | OverridesMarkChildren;
 
     private:
         virtual ConstructType getConstructData(ConstructData&);
         virtual CallType getCallData(CallData&);
+
+        virtual void markChildren(MarkStack&);
 
         virtual const ClassInfo* classInfo() const { return &info; }

trunk/JavaScriptCore/runtime/Structure.h

@@ -95 +95 @@
         const TypeInfo& typeInfo() const { return m_typeInfo; }
 
-        JSValue storedPrototype() const { return m_prototype; }
+        const JSValue& storedPrototype() const { return m_prototype; }
+        JSValue& storedPrototype() { return m_prototype; }
         JSValue prototypeForLookup(ExecState*) const;
         StructureChain* prototypeChain(ExecState*) const;

trunk/JavaScriptGlue/ChangeLog

@@ +1 @@
+2010-08-04  Nathan Lawrence  <nlawrence@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Removed unneeded marking.  We need to remove this marking in order to have
+        MarkStack::append take references for updating movable objects.
+
+        https://bugs.webkit.org/show_bug.cgi?id=41177
+
+        * JSValueWrapper.cpp:
+        (JSValueWrapper::JSObjectMark):
+
 2010-08-03  Gavin Barraclough  <barraclough@apple.com>
 

trunk/JavaScriptGlue/JSValueWrapper.cpp

@@ -193 +193 @@
 }
 
-void JSValueWrapper::JSObjectMark(void *data)
-{
-    JSValueWrapper* ptr = (JSValueWrapper*)data;
-    if (ptr)
-    {
-        // This results in recursive marking but will be otherwise safe and correct.
-        // We claim the array vptr is 0 because we don't have access to it here, and
-        // claiming 0 is functionally harmless -- it merely means that we can't
-        // devirtualise marking of arrays when recursing from this point.
-        MarkStack markStack(0);
-        markStack.append(ptr->fValue.get());
-        markStack.drain();
-    }
-}
+void JSValueWrapper::JSObjectMark(void*)
+{
+    // The object doesn't need to be marked here because it is a protected
+    // object and should therefore be marked by
+    // JSC::Heap::markProtectedObjects.
+
+    // We are keeping the function around because the function is passed as a
+    // callback.
+}