Title: [116494] trunk/Source/_javascript_Core
Revision
116494
Author
[email protected]
Date
2012-05-08 21:45:22 -0700 (Tue, 08 May 2012)

Log Message

ROLLING OUT r114255
        
GC in the middle of JSObject::allocatePropertyStorage can cause badness
https://bugs.webkit.org/show_bug.cgi?id=83839

Reviewed by nobody.

This breaks the world, with COLLECT_ON_EVERY_ALLOCATION enabled.

* _javascript_Core.vcproj/_javascript_Core/_javascript_Core.def:
* jit/JITStubs.cpp:
(JSC::DEFINE_STUB_FUNCTION):
* runtime/JSObject.cpp:
(JSC::JSObject::allocatePropertyStorage):
* runtime/JSObject.h:
(JSObject):
(JSC::JSObject::isUsingInlineStorage):
(JSC):
(JSC::JSObject::putDirectInternal):
(JSC::JSObject::putDirectWithoutTransition):
(JSC::JSObject::transitionTo):
* runtime/Structure.cpp:
(JSC):
* runtime/Structure.h:
(JSC::Structure::didTransition):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (116493 => 116494)


--- trunk/Source/_javascript_Core/ChangeLog	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-05-09 04:45:22 UTC (rev 116494)
@@ -1,3 +1,31 @@
+2012-05-08  Gavin Barraclough  <[email protected]>
+
+        ROLLING OUT r114255
+        
+        GC in the middle of JSObject::allocatePropertyStorage can cause badness
+        https://bugs.webkit.org/show_bug.cgi?id=83839
+
+        Reviewed by nobody.
+
+        This breaks the world, with COLLECT_ON_EVERY_ALLOCATION enabled.
+
+        * _javascript_Core.vcproj/_javascript_Core/_javascript_Core.def:
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::allocatePropertyStorage):
+        * runtime/JSObject.h:
+        (JSObject):
+        (JSC::JSObject::isUsingInlineStorage):
+        (JSC):
+        (JSC::JSObject::putDirectInternal):
+        (JSC::JSObject::putDirectWithoutTransition):
+        (JSC::JSObject::transitionTo):
+        * runtime/Structure.cpp:
+        (JSC):
+        * runtime/Structure.h:
+        (JSC::Structure::didTransition):
+
 2012-05-08  Mark Hahnenberg  <[email protected]>
 
         Heap should not continually allocate new pages in steady state

Modified: trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def (116493 => 116494)


--- trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def	2012-05-09 04:45:22 UTC (rev 116494)
@@ -62,6 +62,7 @@
     ?addSlowCase@Identifier@JSC@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@WTF@@PAVExecState@2@PAVStringImpl@4@@Z
     ?addSlowCase@Identifier@JSC@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@WTF@@PAVJSGlobalData@2@PAVStringImpl@4@@Z
     ?addStaticGlobals@JSGlobalObject@JSC@@IAEXPAUGlobalPropertyInfo@12@H@Z
+    ?allocatePropertyStorage@JSObject@JSC@@QAEXAAVJSGlobalData@2@II@Z
     ?allocateSlowCase@MarkedAllocator@JSC@@AAEPAXXZ
     ?append@StringBuilder@WTF@@QAEXPBEI@Z
     ?append@StringBuilder@WTF@@QAEXPB_WI@Z
@@ -209,7 +210,6 @@
     ?globalExec@JSGlobalObject@JSC@@QAEPAVExecState@2@XZ
     ?globalObjectCount@Heap@JSC@@QAEIXZ
     ?grow@HandleSet@JSC@@AAEXXZ
-    ?growPropertyStorage@JSObject@JSC@@QAEPAV?$WriteBarrierBase@W4Unknown@JSC@@@2@AAVJSGlobalData@2@II@Z
     ?hasInstance@JSObject@JSC@@SA_NPAV12@PAVExecState@2@VJSValue@2@2@Z
     ?hasProperty@JSObject@JSC@@QBE_NPAVExecState@2@ABVIdentifier@2@@Z
     ?hasProperty@JSObject@JSC@@QBE_NPAVExecState@2@I@Z
@@ -320,7 +320,6 @@
     ?stopProfiling@Profiler@JSC@@QAE?AV?$PassRefPtr@VProfile@JSC@@@WTF@@PAVExecState@2@ABVUString@2@@Z
     ?stopSampling@JSGlobalData@JSC@@QAEXXZ
     ?substringSharingImpl@UString@JSC@@QBE?AV12@II@Z
-    ?suggestedNewPropertyStorageSize@Structure@JSC@@QAEIXZ
     ?symbolTableGet@JSVariableObject@JSC@@IAE_NABVIdentifier@2@AAVPropertyDescriptor@2@@Z
     ?synthesizePrototype@JSValue@JSC@@QBEPAVJSObject@2@PAVExecState@2@@Z
     ?thisObject@DebuggerCallFrame@JSC@@QBEPAVJSObject@2@XZ

Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (116493 => 116494)


--- trunk/Source/_javascript_Core/jit/JITStubs.cpp	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp	2012-05-09 04:45:22 UTC (rev 116494)
@@ -1497,9 +1497,7 @@
 
     ASSERT(baseValue.isObject());
     JSObject* base = asObject(baseValue);
-    JSGlobalData& globalData = *stackFrame.globalData;
-    PropertyStorage newStorage = base->growPropertyStorage(globalData, oldSize, newSize);
-    base->setPropertyStorage(globalData, newStorage, newStructure);
+    base->allocatePropertyStorage(*stackFrame.globalData, oldSize, newSize);
 
     return base;
 }

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (116493 => 116494)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2012-05-09 04:45:22 UTC (rev 116494)
@@ -552,7 +552,7 @@
     return m_inheritorID.get();
 }
 
-PropertyStorage JSObject::growPropertyStorage(JSGlobalData& globalData, size_t oldSize, size_t newSize)
+void JSObject::allocatePropertyStorage(JSGlobalData& globalData, size_t oldSize, size_t newSize)
 {
     ASSERT(newSize > oldSize);
 
@@ -580,7 +580,7 @@
     }
 
     ASSERT(newPropertyStorage);
-    return newPropertyStorage;
+    m_propertyStorage.set(globalData, this, newPropertyStorage);
 }
 
 bool JSObject::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)

Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (116493 => 116494)


--- trunk/Source/_javascript_Core/runtime/JSObject.h	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h	2012-05-09 04:45:22 UTC (rev 116494)
@@ -212,9 +212,8 @@
         bool staticFunctionsReified() { return structure()->staticFunctionsReified(); }
         void reifyStaticFunctionsForDelete(ExecState* exec);
 
-        JS_EXPORT_PRIVATE PropertyStorage growPropertyStorage(JSGlobalData&, size_t oldSize, size_t newSize);
+        JS_EXPORT_PRIVATE void allocatePropertyStorage(JSGlobalData&, size_t oldSize, size_t newSize);
         bool isUsingInlineStorage() const { return static_cast<const void*>(m_propertyStorage.get()) == static_cast<const void*>(this + 1); }
-        void setPropertyStorage(JSGlobalData&, PropertyStorage, Structure*);
 
         void* addressOfPropertyStorage()
         {
@@ -453,14 +452,6 @@
     return structure()->typeInfo().type() == GlobalThisType;
 }
 
-inline void JSObject::setPropertyStorage(JSGlobalData& globalData, PropertyStorage storage, Structure* structure)
-{
-    ASSERT(storage);
-    ASSERT(structure);
-    setStructure(globalData, structure);
-    m_propertyStorage.set(globalData, this, storage);
-}
-
 inline JSObject* constructEmptyObject(ExecState* exec, Structure* structure)
 {
     return JSFinalObject::create(exec, structure);
@@ -672,11 +663,10 @@
         if ((mode == PutModePut) && !isExtensible())
             return false;
 
-        PropertyStorage newStorage = propertyStorage();
-        if (structure()->shouldGrowPropertyStorage())
-            newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
+        size_t currentCapacity = structure()->propertyStorageCapacity();
         offset = structure()->addPropertyWithoutTransition(globalData, propertyName, attributes, specificFunction);
-        setPropertyStorage(globalData, newStorage, structure());
+        if (currentCapacity != structure()->propertyStorageCapacity())
+            allocatePropertyStorage(globalData, currentCapacity, structure()->propertyStorageCapacity());
 
         ASSERT(offset < structure()->propertyStorageCapacity());
         putDirectOffset(globalData, offset, value);
@@ -688,13 +678,12 @@
 
     size_t offset;
     size_t currentCapacity = structure()->propertyStorageCapacity();
-    if (Structure* structure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) {
-        PropertyStorage newStorage = propertyStorage(); 
+    if (Structure* structure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) {    
         if (currentCapacity != structure->propertyStorageCapacity())
-            newStorage = growPropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
+            allocatePropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
 
         ASSERT(offset < structure->propertyStorageCapacity());
-        setPropertyStorage(globalData, newStorage, structure);
+        setStructure(globalData, structure);
         putDirectOffset(globalData, offset, value);
         // This is a new property; transitions with specific values are not currently cachable,
         // so leave the slot in an uncachable state.
@@ -738,14 +727,13 @@
     if ((mode == PutModePut) && !isExtensible())
         return false;
 
-    PropertyStorage newStorage = propertyStorage();
-    if (structure()->shouldGrowPropertyStorage())
-        newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
-
     Structure* structure = Structure::addPropertyTransition(globalData, this->structure(), propertyName, attributes, specificFunction, offset);
 
+    if (currentCapacity != structure->propertyStorageCapacity())
+        allocatePropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
+
     ASSERT(offset < structure->propertyStorageCapacity());
-    setPropertyStorage(globalData, newStorage, structure);
+    setStructure(globalData, structure);
     putDirectOffset(globalData, offset, value);
     // This is a new property; transitions with specific values are not currently cachable,
     // so leave the slot in an uncachable state.
@@ -779,20 +767,18 @@
 inline void JSObject::putDirectWithoutTransition(JSGlobalData& globalData, const Identifier& propertyName, JSValue value, unsigned attributes)
 {
     ASSERT(!value.isGetterSetter() && !(attributes & Accessor));
-    PropertyStorage newStorage = propertyStorage();
-    if (structure()->shouldGrowPropertyStorage())
-        newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
+    size_t currentCapacity = structure()->propertyStorageCapacity();
     size_t offset = structure()->addPropertyWithoutTransition(globalData, propertyName, attributes, getJSFunction(value));
-    setPropertyStorage(globalData, newStorage, structure());
+    if (currentCapacity != structure()->propertyStorageCapacity())
+        allocatePropertyStorage(globalData, currentCapacity, structure()->propertyStorageCapacity());
     putDirectOffset(globalData, offset, value);
 }
 
 inline void JSObject::transitionTo(JSGlobalData& globalData, Structure* newStructure)
 {
-    PropertyStorage newStorage = propertyStorage();
     if (structure()->propertyStorageCapacity() != newStructure->propertyStorageCapacity())
-        newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), newStructure->propertyStorageCapacity());
-    setPropertyStorage(globalData, newStorage, newStructure);
+        allocatePropertyStorage(globalData, structure()->propertyStorageCapacity(), newStructure->propertyStorageCapacity());
+    setStructure(globalData, newStructure);
 }
 
 inline JSValue JSObject::toPrimitive(ExecState* exec, PreferredPrimitiveType preferredType) const

Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (116493 => 116494)


--- trunk/Source/_javascript_Core/runtime/Structure.cpp	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp	2012-05-09 04:45:22 UTC (rev 116494)
@@ -267,13 +267,6 @@
         m_propertyStorageCapacity *= 2;
 }
 
-size_t Structure::suggestedNewPropertyStorageSize()
-{
-    if (isUsingInlineStorage())
-        return JSObject::baseExternalStorageCapacity;
-    return m_propertyStorageCapacity * 2;
-}
-
 void Structure::despecifyDictionaryFunction(JSGlobalData& globalData, const Identifier& propertyName)
 {
     StringImpl* rep = propertyName.impl();

Modified: trunk/Source/_javascript_Core/runtime/Structure.h (116493 => 116494)


--- trunk/Source/_javascript_Core/runtime/Structure.h	2012-05-09 04:33:10 UTC (rev 116493)
+++ trunk/Source/_javascript_Core/runtime/Structure.h	2012-05-09 04:45:22 UTC (rev 116494)
@@ -102,8 +102,6 @@
         bool isFrozen(JSGlobalData&);
         bool isExtensible() const { return !m_preventExtensions; }
         bool didTransition() const { return m_didTransition; }
-        bool shouldGrowPropertyStorage() { return propertyStorageCapacity() == propertyStorageSize(); }
-        JS_EXPORT_PRIVATE size_t suggestedNewPropertyStorageSize(); 
 
         Structure* flattenDictionaryStructure(JSGlobalData&, JSObject*);
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to