Title: [154471] trunk/Source/_javascript_Core
Revision
154471
Author
[email protected]
Date
2013-08-22 17:45:38 -0700 (Thu, 22 Aug 2013)

Log Message

JSObject and JSArray code shouldn't have to tiptoe around garbage collection
https://bugs.webkit.org/show_bug.cgi?id=120179

Reviewed by Geoffrey Garen.

There are many places in the code for JSObject and JSArray where they are manipulating their
Butterfly/Structure, e.g. after expanding their out-of-line backing storage via allocating. Within
these places there are certain "critical sections" where a GC would be disastrous. Gen GC looks
like it will make this dance even more intricate. To make everybody's lives easier we should use
the DeferGC mechanism in these functions to make these GC critical sections both obvious in the
code and trivially safe. Deferring collections will usually only last marginally longer, thus we
should not incur any additional overhead.

* heap/Heap.h:
* runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountSlowCase):
* runtime/JSObject.cpp:
(JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
(JSC::JSObject::createInitialUndecided):
(JSC::JSObject::createInitialInt32):
(JSC::JSObject::createInitialDouble):
(JSC::JSObject::createInitialContiguous):
(JSC::JSObject::createArrayStorage):
(JSC::JSObject::convertUndecidedToArrayStorage):
(JSC::JSObject::convertInt32ToArrayStorage):
(JSC::JSObject::convertDoubleToArrayStorage):
(JSC::JSObject::convertContiguousToArrayStorage):
(JSC::JSObject::increaseVectorLength):
(JSC::JSObject::ensureLengthSlow):
* runtime/JSObject.h:
(JSC::JSObject::putDirectInternal):
(JSC::JSObject::setStructureAndReallocateStorageIfNecessary):
(JSC::JSObject::putDirectWithoutTransition):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (154470 => 154471)


--- trunk/Source/_javascript_Core/ChangeLog	2013-08-23 00:28:48 UTC (rev 154470)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-08-23 00:45:38 UTC (rev 154471)
@@ -1,3 +1,39 @@
+2013-08-22  Mark Hahnenberg  <[email protected]>
+
+        JSObject and JSArray code shouldn't have to tiptoe around garbage collection
+        https://bugs.webkit.org/show_bug.cgi?id=120179
+
+        Reviewed by Geoffrey Garen.
+
+        There are many places in the code for JSObject and JSArray where they are manipulating their 
+        Butterfly/Structure, e.g. after expanding their out-of-line backing storage via allocating. Within 
+        these places there are certain "critical sections" where a GC would be disastrous. Gen GC looks 
+        like it will make this dance even more intricate. To make everybody's lives easier we should use 
+        the DeferGC mechanism in these functions to make these GC critical sections both obvious in the 
+        code and trivially safe. Deferring collections will usually only last marginally longer, thus we 
+        should not incur any additional overhead.
+
+        * heap/Heap.h:
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::unshiftCountSlowCase):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::enterDictionaryIndexingModeWhenArrayStorageAlreadyExists):
+        (JSC::JSObject::createInitialUndecided):
+        (JSC::JSObject::createInitialInt32):
+        (JSC::JSObject::createInitialDouble):
+        (JSC::JSObject::createInitialContiguous):
+        (JSC::JSObject::createArrayStorage):
+        (JSC::JSObject::convertUndecidedToArrayStorage):
+        (JSC::JSObject::convertInt32ToArrayStorage):
+        (JSC::JSObject::convertDoubleToArrayStorage):
+        (JSC::JSObject::convertContiguousToArrayStorage):
+        (JSC::JSObject::increaseVectorLength):
+        (JSC::JSObject::ensureLengthSlow):
+        * runtime/JSObject.h:
+        (JSC::JSObject::putDirectInternal):
+        (JSC::JSObject::setStructureAndReallocateStorageIfNecessary):
+        (JSC::JSObject::putDirectWithoutTransition):
+
 2013-08-22  Filip Pizlo  <[email protected]>
 
         Update LLVM binary drops and scripts to the latest version from SVN

Modified: trunk/Source/_javascript_Core/heap/Heap.h (154470 => 154471)


--- trunk/Source/_javascript_Core/heap/Heap.h	2013-08-23 00:28:48 UTC (rev 154470)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2013-08-23 00:45:38 UTC (rev 154471)
@@ -235,9 +235,9 @@
         JSStack& stack();
         BlockAllocator& blockAllocator();
         
-        void incrementDeferralDepth();
+        JS_EXPORT_PRIVATE void incrementDeferralDepth();
         void decrementDeferralDepth();
-        void decrementDeferralDepthAndGCIfNeeded();
+        JS_EXPORT_PRIVATE void decrementDeferralDepthAndGCIfNeeded();
 
         const HeapType m_heapType;
         const size_t m_ramSize;

Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (154470 => 154471)


--- trunk/Source/_javascript_Core/runtime/JSArray.cpp	2013-08-23 00:28:48 UTC (rev 154470)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp	2013-08-23 00:45:38 UTC (rev 154471)
@@ -269,6 +269,7 @@
     // Step 2:
     // We're either going to choose to allocate a new ArrayStorage, or we're going to reuse the existing one.
 
+    DeferGC deferGC(vm.heap);
     void* newAllocBase = 0;
     unsigned newStorageCapacity;
     // If the current storage array is sufficiently large (but not too large!) then just keep using it.

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (154470 => 154471)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2013-08-23 00:28:48 UTC (rev 154470)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2013-08-23 00:45:38 UTC (rev 154471)
@@ -546,6 +546,7 @@
             map->add(this, i).iterator->value.set(vm, this, value);
     }
 
+    DeferGC deferGC(vm.heap);
     Butterfly* newButterfly = storage->butterfly()->resizeArray(vm, this, structure(), 0, ArrayStorage::sizeFor(0));
     RELEASE_ASSERT(newButterfly);
     
@@ -609,6 +610,7 @@
 
 Butterfly* JSObject::createInitialUndecided(VM& vm, unsigned length)
 {
+    DeferGC deferGC(vm.heap);
     Butterfly* newButterfly = createInitialIndexedStorage(vm, length, sizeof(EncodedJSValue));
     Structure* newStructure = Structure::nonPropertyTransition(vm, structure(), AllocateUndecided);
     setStructureAndButterfly(vm, newStructure, newButterfly);
@@ -617,6 +619,7 @@
 
 ContiguousJSValues JSObject::createInitialInt32(VM& vm, unsigned length)
 {
+    DeferGC deferGC(vm.heap);
     Butterfly* newButterfly = createInitialIndexedStorage(vm, length, sizeof(EncodedJSValue));
     Structure* newStructure = Structure::nonPropertyTransition(vm, structure(), AllocateInt32);
     setStructureAndButterfly(vm, newStructure, newButterfly);
@@ -625,6 +628,7 @@
 
 ContiguousDoubles JSObject::createInitialDouble(VM& vm, unsigned length)
 {
+    DeferGC deferGC(vm.heap);
     Butterfly* newButterfly = createInitialIndexedStorage(vm, length, sizeof(double));
     for (unsigned i = newButterfly->vectorLength(); i--;)
         newButterfly->contiguousDouble()[i] = QNaN;
@@ -635,6 +639,7 @@
 
 ContiguousJSValues JSObject::createInitialContiguous(VM& vm, unsigned length)
 {
+    DeferGC deferGC(vm.heap);
     Butterfly* newButterfly = createInitialIndexedStorage(vm, length, sizeof(EncodedJSValue));
     Structure* newStructure = Structure::nonPropertyTransition(vm, structure(), AllocateContiguous);
     setStructureAndButterfly(vm, newStructure, newButterfly);
@@ -643,6 +648,7 @@
 
 ArrayStorage* JSObject::createArrayStorage(VM& vm, unsigned length, unsigned vectorLength)
 {
+    DeferGC deferGC(vm.heap);
     IndexingType oldType = structure()->indexingType();
     ASSERT_UNUSED(oldType, !hasIndexedProperties(oldType));
     Butterfly* newButterfly = Butterfly::createOrGrowArrayRight(
@@ -717,6 +723,7 @@
 
 ArrayStorage* JSObject::convertUndecidedToArrayStorage(VM& vm, NonPropertyTransition transition, unsigned neededLength)
 {
+    DeferGC deferGC(vm.heap);
     ASSERT(hasUndecided(structure()->indexingType()));
     
     ArrayStorage* storage = constructConvertedArrayStorageWithoutCopyingElements(vm, neededLength);
@@ -769,6 +776,7 @@
 {
     ASSERT(hasInt32(structure()->indexingType()));
     
+    DeferGC deferGC(vm.heap);
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, neededLength);
     for (unsigned i = m_butterfly->publicLength(); i--;) {
         JSValue v = m_butterfly->contiguous()[i].get();
@@ -835,6 +843,7 @@
 
 ArrayStorage* JSObject::convertDoubleToArrayStorage(VM& vm, NonPropertyTransition transition, unsigned neededLength)
 {
+    DeferGC deferGC(vm.heap);
     ASSERT(hasDouble(structure()->indexingType()));
     
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, neededLength);
@@ -863,6 +872,7 @@
 
 ArrayStorage* JSObject::convertContiguousToArrayStorage(VM& vm, NonPropertyTransition transition, unsigned neededLength)
 {
+    DeferGC deferGC(vm.heap);
     ASSERT(hasContiguous(structure()->indexingType()));
     
     ArrayStorage* newStorage = constructConvertedArrayStorageWithoutCopyingElements(vm, neededLength);
@@ -2310,6 +2320,7 @@
 
     // Fast case - there is no precapacity. In these cases a realloc makes sense.
     if (LIKELY(!indexBias)) {
+        DeferGC deferGC(vm.heap);
         Butterfly* newButterfly = storage->butterfly()->growArrayRight(
             vm, this, structure(), structure()->outOfLineCapacity(), true,
             ArrayStorage::sizeFor(vectorLength), ArrayStorage::sizeFor(newVectorLength));
@@ -2321,6 +2332,7 @@
     }
     
     // Remove some, but not all of the precapacity. Atomic decay, & capped to not overflow array length.
+    DeferGC deferGC(vm.heap);
     unsigned newIndexBias = std::min(indexBias >> 1, MAX_STORAGE_VECTOR_LENGTH - newVectorLength);
     Butterfly* newButterfly = storage->butterfly()->resizeArray(
         vm, this,
@@ -2345,6 +2357,7 @@
         length << 1,
         MAX_STORAGE_VECTOR_LENGTH);
     unsigned oldVectorLength = m_butterfly->vectorLength();
+    DeferGC deferGC(vm.heap);
     m_butterfly = m_butterfly->growArrayRight(
         vm, this, structure(), structure()->outOfLineCapacity(), true,
         oldVectorLength * sizeof(EncodedJSValue),

Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (154470 => 154471)


--- trunk/Source/_javascript_Core/runtime/JSObject.h	2013-08-23 00:28:48 UTC (rev 154470)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h	2013-08-23 00:45:38 UTC (rev 154471)
@@ -30,6 +30,7 @@
 #include "ClassInfo.h"
 #include "CommonIdentifiers.h"
 #include "CallFrame.h"
+#include "DeferGC.h"
 #include "JSCell.h"
 #include "PropertySlot.h"
 #include "PropertyStorage.h"
@@ -1290,6 +1291,7 @@
         if ((mode == PutModePut) && !isExtensible())
             return false;
 
+        DeferGC deferGC(vm.heap);
         Butterfly* newButterfly = m_butterfly;
         if (structure()->putWillGrowOutOfLineStorage())
             newButterfly = growOutOfLineStorage(vm, structure()->outOfLineCapacity(), structure()->suggestedNewOutOfLineStorageCapacity());
@@ -1310,6 +1312,7 @@
     PropertyOffset offset;
     size_t currentCapacity = structure()->outOfLineCapacity();
     if (Structure* structure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) {
+        DeferGC deferGC(vm.heap);
         Butterfly* newButterfly = m_butterfly;
         if (currentCapacity != structure->outOfLineCapacity())
             newButterfly = growOutOfLineStorage(vm, currentCapacity, structure->outOfLineCapacity());
@@ -1384,7 +1387,8 @@
         setStructure(vm, newStructure);
         return;
     }
-    
+
+    DeferGC deferGC(vm.heap); 
     Butterfly* newButterfly = growOutOfLineStorage(
         vm, oldCapacity, newStructure->outOfLineCapacity());
     setStructureAndButterfly(vm, newStructure, newButterfly);
@@ -1420,6 +1424,7 @@
 
 inline void JSObject::putDirectWithoutTransition(VM& vm, PropertyName propertyName, JSValue value, unsigned attributes)
 {
+    DeferGC deferGC(vm.heap);
     ASSERT(!value.isGetterSetter() && !(attributes & Accessor));
     Butterfly* newButterfly = m_butterfly;
     if (structure()->putWillGrowOutOfLineStorage())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to