Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more...
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Mar 2015 21:29:57 +0000 (21:29 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Mar 2015 21:29:57 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142589

Reviewed by Andreas Kling.

Source/JavaScriptCore:

* API/JSBase.cpp:
(JSReportExtraMemoryCost): Added a FIXME to annotate a known bug.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::CodeBlock):
(JSC::CodeBlock::visitAggregate):
* bytecode/CodeBlock.h:
(JSC::CodeBlock::setJITCode): Updated for rename.

* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::reportExtraMemoryAllocatedSlowCase):
(JSC::Heap::deprecatedReportExtraMemorySlowCase): Renamed our reporting
APIs to clarify their relationship to each other: One must report extra
memory at the time of allocation, and at the time the GC visits it.

(JSC::Heap::extraMemorySize):
(JSC::Heap::size):
(JSC::Heap::capacity):
(JSC::Heap::sizeAfterCollect):
(JSC::Heap::willStartCollection): Updated for renames. Added explicit
API for deprecated users who can't use our best API.

(JSC::Heap::reportExtraMemoryCostSlowCase): Deleted.
(JSC::Heap::extraSize): Deleted.

* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::reportExtraMemoryAllocated):
(JSC::Heap::reportExtraMemoryVisited):
(JSC::Heap::deprecatedReportExtraMemory):
(JSC::Heap::reportExtraMemoryCost): Deleted. Ditto.

* heap/SlotVisitor.h:
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::reportExtraMemoryVisited):
(JSC::SlotVisitor::reportExtraMemoryUsage): Deleted. Moved this
functionality into the Heap since it's pretty detailed in its access
to the heap.

* runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::visitChildren): Updated for
renames.

* runtime/JSString.cpp:
(JSC::JSString::visitChildren):
(JSC::JSRopeString::resolveRopeToAtomicString):
(JSC::JSRopeString::resolveRope):
* runtime/JSString.h:
(JSC::JSString::finishCreation): Updated for renames.

* runtime/SparseArrayValueMap.cpp:
(JSC::SparseArrayValueMap::add): Added FIXME.

* runtime/WeakMapData.cpp:
(JSC::WeakMapData::visitChildren): Updated for rename.

Source/WebCore:

Updated for renames to JSC extra cost APIs.

Added FIXMEs to our 10 use cases that are currently wrong, including
canvas, which is the cause of https://bugs.webkit.org/show_bug.cgi?id=142457.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::appendBufferInternal):
(WebCore::SourceBuffer::sourceBufferPrivateAppendComplete):
(WebCore::SourceBuffer::reportExtraMemoryAllocated):
(WebCore::SourceBuffer::reportExtraMemoryCost): Deleted.
* Modules/mediasource/SourceBuffer.h:
* bindings/js/JSDocumentCustom.cpp:
(WebCore::toJS):
* bindings/js/JSImageDataCustom.cpp:
(WebCore::toJS):
* bindings/js/JSNodeListCustom.cpp:
(WebCore::createWrapper):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
* dom/CollectionIndexCache.cpp:
(WebCore::reportExtraMemoryAllocatedForCollectionIndexCache):
(WebCore::reportExtraMemoryCostForCollectionIndexCache): Deleted.
* dom/CollectionIndexCache.h:
(WebCore::Iterator>::computeNodeCountUpdatingListCache):
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::createImageBuffer):
* html/HTMLCollection.h:
(WebCore::CollectionNamedElementCache::didPopulate):
* html/HTMLImageLoader.cpp:
(WebCore::HTMLImageLoader::imageChanged):
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::parseAttribute):
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::dropProtection):

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

29 files changed:
Source/JavaScriptCore/API/JSBase.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/heap/SlotVisitor.h
Source/JavaScriptCore/heap/SlotVisitorInlines.h
Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
Source/JavaScriptCore/runtime/JSString.cpp
Source/JavaScriptCore/runtime/JSString.h
Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp
Source/JavaScriptCore/runtime/WeakMapData.cpp
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Source/WebCore/Modules/mediasource/SourceBuffer.h
Source/WebCore/bindings/js/JSDocumentCustom.cpp
Source/WebCore/bindings/js/JSImageDataCustom.cpp
Source/WebCore/bindings/js/JSNodeListCustom.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/dom/CollectionIndexCache.cpp
Source/WebCore/dom/CollectionIndexCache.h
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/HTMLCollection.h
Source/WebCore/html/HTMLImageLoader.cpp
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/xml/XMLHttpRequest.cpp

index e4f5946..7fc0f94 100644 (file)
@@ -139,7 +139,10 @@ void JSReportExtraMemoryCost(JSContextRef ctx, size_t size)
     }
     ExecState* exec = toJS(ctx);
     JSLockHolder locker(exec);
-    exec->vm().heap.reportExtraMemoryCost(size);
+
+    // FIXME: switch to deprecatedReportExtraMemory.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    exec->vm().heap.reportExtraMemoryAllocated(size);
 }
 
 extern "C" JS_EXPORT void JSSynchronousGarbageCollectForDebugging(JSContextRef);
index f78434a..095cb04 100644 (file)
@@ -1,3 +1,69 @@
+2015-03-11  Geoffrey Garen  <ggaren@apple.com>
+
+        Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
+        https://bugs.webkit.org/show_bug.cgi?id=142589
+
+        Reviewed by Andreas Kling.
+
+        * API/JSBase.cpp:
+        (JSReportExtraMemoryCost): Added a FIXME to annotate a known bug.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::CodeBlock):
+        (JSC::CodeBlock::visitAggregate):
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::setJITCode): Updated for rename.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::reportExtraMemoryAllocatedSlowCase):
+        (JSC::Heap::deprecatedReportExtraMemorySlowCase): Renamed our reporting
+        APIs to clarify their relationship to each other: One must report extra
+        memory at the time of allocation, and at the time the GC visits it.
+
+        (JSC::Heap::extraMemorySize):
+        (JSC::Heap::size):
+        (JSC::Heap::capacity):
+        (JSC::Heap::sizeAfterCollect):
+        (JSC::Heap::willStartCollection): Updated for renames. Added explicit
+        API for deprecated users who can't use our best API.
+        (JSC::Heap::reportExtraMemoryCostSlowCase): Deleted.
+        (JSC::Heap::extraSize): Deleted.
+
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::reportExtraMemoryAllocated):
+        (JSC::Heap::reportExtraMemoryVisited):
+        (JSC::Heap::deprecatedReportExtraMemory):
+        (JSC::Heap::reportExtraMemoryCost): Deleted. Ditto.
+
+        * heap/SlotVisitor.h:
+        * heap/SlotVisitorInlines.h:
+        (JSC::SlotVisitor::reportExtraMemoryVisited):
+        (JSC::SlotVisitor::reportExtraMemoryUsage): Deleted. Moved this
+        functionality into the Heap since it's pretty detailed in its access
+        to the heap.
+
+        * runtime/JSArrayBufferView.cpp:
+        (JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::visitChildren): Updated for
+        renames.
+
+        * runtime/JSString.cpp:
+        (JSC::JSString::visitChildren):
+        (JSC::JSRopeString::resolveRopeToAtomicString):
+        (JSC::JSRopeString::resolveRope):
+        * runtime/JSString.h:
+        (JSC::JSString::finishCreation): Updated for renames.
+
+        * runtime/SparseArrayValueMap.cpp:
+        (JSC::SparseArrayValueMap::add): Added FIXME.
+
+        * runtime/WeakMapData.cpp:
+        (JSC::WeakMapData::visitChildren): Updated for rename.
+
 2015-03-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Calling super() in a base class results in a crash
index d26ee2b..68cd3ae 100644 (file)
@@ -1677,7 +1677,7 @@ CodeBlock::CodeBlock(CopyParsedBlockTag, CodeBlock& other)
     }
     
     m_heap->m_codeBlocks.add(this);
-    m_heap->reportExtraMemoryCost(sizeof(CodeBlock));
+    m_heap->reportExtraMemoryAllocated(sizeof(CodeBlock));
 }
 
 CodeBlock::CodeBlock(ScriptExecutable* ownerExecutable, UnlinkedCodeBlock* unlinkedCodeBlock, JSScope* scope, PassRefPtr<SourceProvider> sourceProvider, unsigned sourceOffset, unsigned firstLineColumnOffset)
@@ -2131,7 +2131,7 @@ CodeBlock::CodeBlock(ScriptExecutable* ownerExecutable, UnlinkedCodeBlock* unlin
         dumpBytecode();
     
     m_heap->m_codeBlocks.add(this);
-    m_heap->reportExtraMemoryCost(sizeof(CodeBlock) + m_instructions.size() * sizeof(Instruction));
+    m_heap->reportExtraMemoryAllocated(sizeof(CodeBlock) + m_instructions.size() * sizeof(Instruction));
 }
 
 CodeBlock::~CodeBlock()
@@ -2223,15 +2223,15 @@ void CodeBlock::visitAggregate(SlotVisitor& visitor)
     if (CodeBlock* otherBlock = specialOSREntryBlockOrNull())
         otherBlock->visitAggregate(visitor);
 
-    visitor.reportExtraMemoryUsage(ownerExecutable(), sizeof(CodeBlock));
+    visitor.reportExtraMemoryVisited(ownerExecutable(), sizeof(CodeBlock));
     if (m_jitCode)
-        visitor.reportExtraMemoryUsage(ownerExecutable(), m_jitCode->size());
+        visitor.reportExtraMemoryVisited(ownerExecutable(), m_jitCode->size());
     if (m_instructions.size()) {
         // Divide by refCount() because m_instructions points to something that is shared
         // by multiple CodeBlocks, and we only want to count it towards the heap size once.
         // Having each CodeBlock report only its proportional share of the size is one way
         // of accomplishing this.
-        visitor.reportExtraMemoryUsage(ownerExecutable(), m_instructions.size() * sizeof(Instruction) / m_instructions.refCount());
+        visitor.reportExtraMemoryVisited(ownerExecutable(), m_instructions.size() * sizeof(Instruction) / m_instructions.refCount());
     }
 
     visitor.append(&m_unlinkedCode);
index 7763bb9..1eb91ef 100644 (file)
@@ -273,7 +273,7 @@ public:
     void setJITCode(PassRefPtr<JITCode> code)
     {
         ASSERT(m_heap->isDeferred());
-        m_heap->reportExtraMemoryCost(code->size());
+        m_heap->reportExtraMemoryAllocated(code->size());
         ConcurrentJITLocker locker(m_lock);
         WTF::storeStoreFence(); // This is probably not needed because the lock will also do something similar, but it's good to be paranoid.
         m_jitCode = code;
index 36da6f8..5a79eda 100644 (file)
@@ -309,7 +309,8 @@ Heap::Heap(VM* vm, HeapType heapType)
     , m_operationInProgress(NoOperation)
     , m_objectSpace(this)
     , m_storageSpace(this)
-    , m_extraMemoryUsage(0)
+    , m_extraMemorySize(0)
+    , m_deprecatedExtraMemorySize(0)
     , m_machineThreads(this)
     , m_sharedData(vm)
     , m_slotVisitor(m_sharedData)
@@ -392,23 +393,18 @@ void Heap::releaseDelayedReleasedObjects()
 #endif
 }
 
-void Heap::reportExtraMemoryCostSlowCase(size_t cost)
+void Heap::reportExtraMemoryAllocatedSlowCase(size_t size)
 {
-    // Our frequency of garbage collection tries to balance memory use against speed
-    // by collecting based on the number of newly created values. However, for values
-    // that hold on to a great deal of memory that's not in the form of other JS values,
-    // that is not good enough - in some cases a lot of those objects can pile up and
-    // use crazy amounts of memory without a GC happening. So we track these extra
-    // memory costs. Only unusually large objects are noted, and we only keep track
-    // of this extra cost until the next GC. In garbage collected languages, most values
-    // are either very short lived temporaries, or have extremely long lifetimes. So
-    // if a large value survives one garbage collection, there is not much point to
-    // collecting more frequently as long as it stays alive.
-
-    didAllocate(cost);
+    didAllocate(size);
     collectIfNecessaryOrDefer();
 }
 
+void Heap::deprecatedReportExtraMemorySlowCase(size_t size)
+{
+    m_deprecatedExtraMemorySize += size;
+    reportExtraMemoryAllocatedSlowCase(size);
+}
+
 void Heap::reportAbandonedObjectGraph()
 {
     // Our clients don't know exactly how much memory they
@@ -854,19 +850,19 @@ size_t Heap::objectCount()
     return m_objectSpace.objectCount();
 }
 
-size_t Heap::extraSize()
+size_t Heap::extraMemorySize()
 {
-    return m_extraMemoryUsage + m_arrayBuffers.size();
+    return m_extraMemorySize + m_deprecatedExtraMemorySize + m_arrayBuffers.size();
 }
 
 size_t Heap::size()
 {
-    return m_objectSpace.size() + m_storageSpace.size() + extraSize();
+    return m_objectSpace.size() + m_storageSpace.size() + extraMemorySize();
 }
 
 size_t Heap::capacity()
 {
-    return m_objectSpace.capacity() + m_storageSpace.capacity() + extraSize();
+    return m_objectSpace.capacity() + m_storageSpace.capacity() + extraMemorySize();
 }
 
 size_t Heap::sizeAfterCollect()
@@ -876,7 +872,7 @@ size_t Heap::sizeAfterCollect()
     // rather than all used (including dead) copied bytes, thus it's 
     // always the case that m_totalBytesCopied <= m_storageSpace.size(). 
     ASSERT(m_totalBytesCopied <= m_storageSpace.size());
-    return m_totalBytesVisited + m_totalBytesCopied + extraSize();
+    return m_totalBytesVisited + m_totalBytesCopied + extraMemorySize();
 }
 
 size_t Heap::protectedGlobalObjectCount()
@@ -1124,7 +1120,8 @@ void Heap::willStartCollection(HeapOperation collectionType)
     }
     if (m_operationInProgress == FullCollection) {
         m_sizeBeforeLastFullCollect = m_sizeAfterLastCollect + m_bytesAllocatedThisCycle;
-        m_extraMemoryUsage = 0;
+        m_extraMemorySize = 0;
+        m_deprecatedExtraMemorySize = 0;
 
         if (m_fullActivityCallback)
             m_fullActivityCallback->willCollect();
index e589ec0..9833ad4 100644 (file)
@@ -161,13 +161,21 @@ public:
     JS_EXPORT_PRIVATE void collect(HeapOperation collectionType = AnyCollection);
     bool collectIfNecessaryOrDefer(); // Returns true if it did collect.
 
-    void reportExtraMemoryCost(size_t cost);
+    // Use this API to report non-GC memory referenced by GC objects. Be sure to
+    // call both of these functions: Calling only one may trigger catastropic
+    // memory growth.
+    void reportExtraMemoryAllocated(size_t);
+    void reportExtraMemoryVisited(JSCell*, size_t);
+
+    // Use this API to report non-GC memory if you can't use the better API above.
+    void deprecatedReportExtraMemory(size_t);
+
     JS_EXPORT_PRIVATE void reportAbandonedObjectGraph();
 
     JS_EXPORT_PRIVATE void protect(JSValue);
     JS_EXPORT_PRIVATE bool unprotect(JSValue); // True when the protect count drops to 0.
     
-    size_t extraSize(); // extra memory usage outside of pages allocated by the heap
+    size_t extraMemorySize(); // Non-GC memory referenced by GC objects.
     JS_EXPORT_PRIVATE size_t size();
     JS_EXPORT_PRIVATE size_t capacity();
     JS_EXPORT_PRIVATE size_t objectCount();
@@ -261,15 +269,15 @@ private:
     void* allocateWithoutDestructor(size_t); // For use with objects without destructors.
     template<typename ClassType> void* allocateObjectOfType(size_t); // Chooses one of the methods above based on type.
 
-    static const size_t minExtraCost = 256;
-    static const size_t maxExtraCost = 1024 * 1024;
+    static const size_t minExtraMemory = 256;
     
     class FinalizerOwner : public WeakHandleOwner {
         virtual void finalize(Handle<Unknown>, void* context) override;
     };
 
     JS_EXPORT_PRIVATE bool isValidAllocation(size_t);
-    JS_EXPORT_PRIVATE void reportExtraMemoryCostSlowCase(size_t);
+    JS_EXPORT_PRIVATE void reportExtraMemoryAllocatedSlowCase(size_t);
+    JS_EXPORT_PRIVATE void deprecatedReportExtraMemorySlowCase(size_t);
 
     void collectImpl(HeapOperation, void* stackOrigin, void* stackTop, MachineThreads::RegisterState&);
 
@@ -353,7 +361,8 @@ private:
     MarkedSpace m_objectSpace;
     CopiedSpace m_storageSpace;
     GCIncomingRefCountedSet<ArrayBuffer> m_arrayBuffers;
-    size_t m_extraMemoryUsage;
+    size_t m_extraMemorySize;
+    size_t m_deprecatedExtraMemorySize;
 
     HashSet<const JSCell*> m_copyingRememberedSet;
 
index 58e92d4..07eec11 100644 (file)
@@ -152,10 +152,39 @@ inline void Heap::writeBarrier(const JSCell* from)
 #endif
 }
 
-inline void Heap::reportExtraMemoryCost(size_t cost)
+inline void Heap::reportExtraMemoryAllocated(size_t size)
 {
-    if (cost > minExtraCost) 
-        reportExtraMemoryCostSlowCase(cost);
+    if (size > minExtraMemory) 
+        reportExtraMemoryAllocatedSlowCase(size);
+}
+
+inline void Heap::reportExtraMemoryVisited(JSCell* owner, size_t size)
+{
+#if ENABLE(GGC)
+    // We don't want to double-count the extra memory that was reported in previous collections.
+    if (operationInProgress() == EdenCollection && Heap::isRemembered(owner))
+        return;
+#else
+    UNUSED_PARAM(owner);
+#endif
+
+    size_t* counter = &m_extraMemorySize;
+    
+#if ENABLE(COMPARE_AND_SWAP)
+    for (;;) {
+        size_t oldSize = *counter;
+        if (WTF::weakCompareAndSwapSize(counter, oldSize, oldSize + size))
+            return;
+    }
+#else
+    (*counter) += size;
+#endif
+}
+
+inline void Heap::deprecatedReportExtraMemory(size_t size)
+{
+    if (size > minExtraMemory) 
+        deprecatedReportExtraMemorySlowCase(size);
 }
 
 template<typename Functor> inline typename Functor::ReturnType Heap::forEachProtectedCell(Functor& functor)
index 25e32ea..5152be2 100644 (file)
@@ -105,7 +105,7 @@ public:
 
     void copyLater(JSCell*, CopyToken, void*, size_t);
     
-    void reportExtraMemoryUsage(JSCell* owner, size_t);
+    void reportExtraMemoryVisited(JSCell* owner, size_t);
     
     void addWeakReferenceHarvester(WeakReferenceHarvester*);
     void addUnconditionalFinalizer(UnconditionalFinalizer*);
index f3355a5..9b62556 100644 (file)
@@ -252,27 +252,9 @@ inline void SlotVisitor::copyLater(JSCell* owner, CopyToken token, void* ptr, si
     }
 }
     
-inline void SlotVisitor::reportExtraMemoryUsage(JSCell* owner, size_t size)
+inline void SlotVisitor::reportExtraMemoryVisited(JSCell* owner, size_t size)
 {
-#if ENABLE(GGC)
-    // We don't want to double-count the extra memory that was reported in previous collections.
-    if (heap()->operationInProgress() == EdenCollection && Heap::isRemembered(owner))
-        return;
-#else
-    UNUSED_PARAM(owner);
-#endif
-
-    size_t* counter = &m_shared.m_vm->heap.m_extraMemoryUsage;
-    
-#if ENABLE(COMPARE_AND_SWAP)
-    for (;;) {
-        size_t oldSize = *counter;
-        if (WTF::weakCompareAndSwapSize(counter, oldSize, oldSize + size))
-            return;
-    }
-#else
-    (*counter) += size;
-#endif
+    heap()->reportExtraMemoryVisited(owner, size);
 }
 
 inline Heap* SlotVisitor::heap() const
index cb5c8a2..0a116c0 100644 (file)
@@ -78,7 +78,7 @@ JSArrayBufferView::ConstructionContext::ConstructionContext(
             return;
     }
     
-    vm.heap.reportExtraMemoryCost(static_cast<size_t>(length) * elementSize);
+    vm.heap.reportExtraMemoryAllocated(static_cast<size_t>(length) * elementSize);
     
     m_structure = structure;
     m_mode = OversizeTypedArray;
index 6d0d21f..401b8da 100644 (file)
@@ -446,7 +446,7 @@ void JSGenericTypedArrayView<Adaptor>::visitChildren(JSCell* cell, SlotVisitor&
     }
         
     case OversizeTypedArray: {
-        visitor.reportExtraMemoryUsage(thisObject, thisObject->byteSize());
+        visitor.reportExtraMemoryVisited(thisObject, thisObject->byteSize());
         break;
     }
         
index edd1cf5..f6d84bc 100644 (file)
@@ -77,7 +77,7 @@ void JSString::visitChildren(JSCell* cell, SlotVisitor& visitor)
     else {
         StringImpl* impl = thisObject->m_value.impl();
         ASSERT(impl);
-        visitor.reportExtraMemoryUsage(thisObject, impl->costDuringGC());
+        visitor.reportExtraMemoryVisited(thisObject, impl->costDuringGC());
     }
 }
 
@@ -181,7 +181,7 @@ void JSRopeString::resolveRopeToAtomicString(ExecState* exec) const
 
     // If we resolved a string that didn't previously exist, notify the heap that we've grown.
     if (m_value.impl()->hasOneRef())
-        Heap::heap(this)->reportExtraMemoryCost(m_value.impl()->cost());
+        Heap::heap(this)->reportExtraMemoryAllocated(m_value.impl()->cost());
 }
 
 void JSRopeString::clearFibers() const
@@ -240,7 +240,7 @@ void JSRopeString::resolveRope(ExecState* exec) const
     if (is8Bit()) {
         LChar* buffer;
         if (RefPtr<StringImpl> newImpl = StringImpl::tryCreateUninitialized(m_length, buffer)) {
-            Heap::heap(this)->reportExtraMemoryCost(newImpl->cost());
+            Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost());
             m_value = newImpl.release();
         } else {
             outOfMemory(exec);
@@ -254,7 +254,7 @@ void JSRopeString::resolveRope(ExecState* exec) const
 
     UChar* buffer;
     if (RefPtr<StringImpl> newImpl = StringImpl::tryCreateUninitialized(m_length, buffer)) {
-        Heap::heap(this)->reportExtraMemoryCost(newImpl->cost());
+        Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost());
         m_value = newImpl.release();
     } else {
         outOfMemory(exec);
index 0575447..281ae46 100644 (file)
@@ -107,7 +107,7 @@ private:
         Base::finishCreation(vm);
         m_length = length;
         setIs8Bit(m_value.impl()->is8Bit());
-        Heap::heap(this)->reportExtraMemoryCost(cost);
+        Heap::heap(this)->reportExtraMemoryAllocated(cost);
         vm.m_newStringsSinceLastHashCons++;
     }
 
index 2a145da..7c784ff 100644 (file)
@@ -80,7 +80,9 @@ SparseArrayValueMap::AddResult SparseArrayValueMap::add(JSObject* array, unsigne
     AddResult result = m_map.add(i, entry);
     size_t capacity = m_map.capacity();
     if (capacity != m_reportedCapacity) {
-        Heap::heap(array)->reportExtraMemoryCost((capacity - m_reportedCapacity) * (sizeof(unsigned) + sizeof(WriteBarrier<Unknown>)));
+        // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+        // https://bugs.webkit.org/show_bug.cgi?id=142593
+        Heap::heap(array)->reportExtraMemoryAllocated((capacity - m_reportedCapacity) * (sizeof(unsigned) + sizeof(WriteBarrier<Unknown>)));
         m_reportedCapacity = capacity;
     }
     return result;
index dcdaf21..8ee9de7 100644 (file)
@@ -64,7 +64,7 @@ void WeakMapData::visitChildren(JSCell* cell, SlotVisitor& visitor)
     // Rough approximation of the external storage needed for the hashtable.
     // This isn't exact, but it is close enough, and proportional to the actual
     // external mermory usage.
-    visitor.reportExtraMemoryUsage(thisObj, thisObj->m_map.capacity() * (sizeof(JSObject*) + sizeof(WriteBarrier<Unknown>)));
+    visitor.reportExtraMemoryVisited(thisObj, thisObj->m_map.capacity() * (sizeof(JSObject*) + sizeof(WriteBarrier<Unknown>)));
 }
 
 void WeakMapData::set(VM& vm, JSObject* key, JSValue value)
index d8a7630..92bb929 100644 (file)
@@ -1,3 +1,45 @@
+2015-03-11  Geoffrey Garen  <ggaren@apple.com>
+
+        Refactored the JSC::Heap extra cost API for clarity and to make some known bugs more obvious
+        https://bugs.webkit.org/show_bug.cgi?id=142589
+
+        Reviewed by Andreas Kling.
+
+        Updated for renames to JSC extra cost APIs.
+
+        Added FIXMEs to our 10 use cases that are currently wrong, including
+        canvas, which is the cause of https://bugs.webkit.org/show_bug.cgi?id=142457.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::appendBufferInternal):
+        (WebCore::SourceBuffer::sourceBufferPrivateAppendComplete):
+        (WebCore::SourceBuffer::reportExtraMemoryAllocated):
+        (WebCore::SourceBuffer::reportExtraMemoryCost): Deleted.
+        * Modules/mediasource/SourceBuffer.h:
+        * bindings/js/JSDocumentCustom.cpp:
+        (WebCore::toJS):
+        * bindings/js/JSImageDataCustom.cpp:
+        (WebCore::toJS):
+        * bindings/js/JSNodeListCustom.cpp:
+        (WebCore::createWrapper):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation):
+        * dom/CollectionIndexCache.cpp:
+        (WebCore::reportExtraMemoryAllocatedForCollectionIndexCache):
+        (WebCore::reportExtraMemoryCostForCollectionIndexCache): Deleted.
+        * dom/CollectionIndexCache.h:
+        (WebCore::Iterator>::computeNodeCountUpdatingListCache):
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::createImageBuffer):
+        * html/HTMLCollection.h:
+        (WebCore::CollectionNamedElementCache::didPopulate):
+        * html/HTMLImageLoader.cpp:
+        (WebCore::HTMLImageLoader::imageChanged):
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::dropProtection):
+
 2015-03-11  Benjamin Poulain  <bpoulain@apple.com>
 
         Add basic support for BOL and EOL assertions to the URL Filter parser
index 59b5420..3f16298 100644 (file)
@@ -581,7 +581,7 @@ void SourceBuffer::appendBufferInternal(unsigned char* data, unsigned size, Exce
     // 6. Asynchronously run the buffer append algorithm.
     m_appendBufferTimer.startOneShot(0);
 
-    reportExtraMemoryCost();
+    reportExtraMemoryAllocated();
 }
 
 void SourceBuffer::appendBufferTimerFired()
@@ -667,7 +667,7 @@ void SourceBuffer::sourceBufferPrivateAppendComplete(SourceBufferPrivate*, Appen
             provideMediaData(trackBuffer, trackID);
     }
 
-    reportExtraMemoryCost();
+    reportExtraMemoryAllocated();
     if (extraMemoryCost() > this->maximumBufferSize())
         m_bufferFull = true;
 
@@ -1988,7 +1988,7 @@ size_t SourceBuffer::extraMemoryCost() const
     return extraMemoryCost;
 }
 
-void SourceBuffer::reportExtraMemoryCost()
+void SourceBuffer::reportExtraMemoryAllocated()
 {
     size_t extraMemoryCost = this->extraMemoryCost();
     if (extraMemoryCost < m_reportedExtraMemoryCost)
@@ -1998,8 +1998,11 @@ void SourceBuffer::reportExtraMemoryCost()
     m_reportedExtraMemoryCost = extraMemoryCost;
 
     JSC::JSLockHolder lock(scriptExecutionContext()->vm());
-    if (extraMemoryCostDelta > 0)
-        scriptExecutionContext()->vm().heap.reportExtraMemoryCost(extraMemoryCostDelta);
+    if (extraMemoryCostDelta > 0) {
+        // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+        // https://bugs.webkit.org/show_bug.cgi?id=142593
+        scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(extraMemoryCostDelta);
+    }
 }
 
 Vector<String> SourceBuffer::bufferedSamplesForTrackID(const AtomicString& trackID)
index 49269ff..905ea38 100644 (file)
@@ -192,7 +192,7 @@ private:
     void removeCodedFrames(const MediaTime& start, const MediaTime& end);
 
     size_t extraMemoryCost() const;
-    void reportExtraMemoryCost();
+    void reportExtraMemoryAllocated();
 
     std::unique_ptr<PlatformTimeRanges> bufferedAccountingForEndOfStream() const;
 
index 813eeb9..cd19a3d 100644 (file)
@@ -109,7 +109,9 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, Document* documen
         for (Node* n = document; n; n = NodeTraversal::next(*n))
             nodeCount++;
         
-        exec->heap()->reportExtraMemoryCost(nodeCount * sizeof(Node));
+        // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+        // https://bugs.webkit.org/show_bug.cgi?id=142593
+        exec->heap()->reportExtraMemoryAllocated(nodeCount * sizeof(Node));
     }
 
     return wrapper;
index 5aae5ae..aa8766d 100644 (file)
@@ -47,7 +47,9 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, ImageData* imageD
     wrapper = CREATE_DOM_WRAPPER(globalObject, ImageData, imageData);
     Identifier dataName(exec, "data");
     wrapper->putDirect(exec->vm(), dataName, toJS(exec, globalObject, imageData->data()), DontDelete | ReadOnly);
-    exec->heap()->reportExtraMemoryCost(imageData->data()->length());
+    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    exec->heap()->reportExtraMemoryAllocated(imageData->data()->length());
     
     return wrapper;
 }
index 7b62ecf..9887dfc 100644 (file)
@@ -62,7 +62,9 @@ bool JSNodeList::getOwnPropertySlotDelegate(ExecState* exec, PropertyName proper
 
 JSC::JSValue createWrapper(JSDOMGlobalObject& globalObject, NodeList& nodeList)
 {
-    globalObject.vm().heap.reportExtraMemoryCost(nodeList.memoryCost());
+    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    globalObject.vm().heap.reportExtraMemoryAllocated(nodeList.memoryCost());
     return createNewWrapper<JSNodeList>(&globalObject, &nodeList);
 }
 
index afc0ec0..507fba0 100644 (file)
@@ -2865,7 +2865,7 @@ sub GenerateImplementation
         }
         push(@implContent, "    thisObject->visitAdditionalChildren(visitor);\n") if $interface->extendedAttributes->{"JSCustomMarkFunction"};
         if ($interface->extendedAttributes->{"ReportExtraMemoryCost"}) {
-            push(@implContent, "    visitor.reportExtraMemoryUsage(cell, thisObject->impl().memoryCost());\n");
+            push(@implContent, "    visitor.reportExtraMemoryVisited(cell, thisObject->impl().memoryCost());\n");
         }
         if ($numCachedAttributes > 0) {
             foreach (@{$interface->attributes}) {
@@ -3080,7 +3080,7 @@ END
 #endif
 END
         push(@implContent, <<END) if $interface->extendedAttributes->{"ReportExtraMemoryCost"};
-    globalObject->vm().heap.reportExtraMemoryCost(impl->memoryCost());
+    globalObject->vm().heap.reportExtraMemoryAllocated(impl->memoryCost());
 END
 
         if ($svgPropertyType) {
index 3bfa907..cc81e0a 100644 (file)
 
 namespace WebCore {
 
-void reportExtraMemoryCostForCollectionIndexCache(size_t cost)
+void reportExtraMemoryAllocatedForCollectionIndexCache(size_t cost)
 {
     JSC::VM& vm = JSDOMWindowBase::commonVM();
     JSC::JSLockHolder lock(vm);
-    vm.heap.reportExtraMemoryCost(cost);
+    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    vm.heap.reportExtraMemoryAllocated(cost);
 }
 
 }
index 6051977..5821327 100644 (file)
@@ -30,7 +30,7 @@
 
 namespace WebCore {
 
-void reportExtraMemoryCostForCollectionIndexCache(size_t);
+void reportExtraMemoryAllocatedForCollectionIndexCache(size_t);
 
 template <class Collection, class Iterator>
 class CollectionIndexCache {
@@ -100,7 +100,7 @@ unsigned CollectionIndexCache<Collection, Iterator>::computeNodeCountUpdatingLis
     m_listValid = true;
 
     if (unsigned capacityDifference = m_cachedList.capacity() - oldCapacity)
-        reportExtraMemoryCostForCollectionIndexCache(capacityDifference * sizeof(NodeType*));
+        reportExtraMemoryAllocatedForCollectionIndexCache(capacityDifference * sizeof(NodeType*));
 
     return m_cachedList.size();
 }
index fc30c23..c2a08bf 100644 (file)
@@ -576,7 +576,9 @@ void HTMLCanvasElement::createImageBuffer() const
 
     JSC::JSLockHolder lock(scriptExecutionContext()->vm());
     size_t numBytes = 4 * m_imageBuffer->internalSize().width() * m_imageBuffer->internalSize().height();
-    scriptExecutionContext()->vm().heap.reportExtraMemoryCost(numBytes);
+    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(numBytes);
 
 #if USE(IOSURFACE_CANVAS_BACKING_STORE) || ENABLE(ACCELERATED_2D_CANVAS)
     if (m_context && m_context->is2d())
index beb26d1..4714e90 100644 (file)
@@ -160,7 +160,7 @@ inline void CollectionNamedElementCache::didPopulate()
     m_didPopulate = true;
 #endif
     if (size_t cost = memoryCost())
-        reportExtraMemoryCostForCollectionIndexCache(cost);
+        reportExtraMemoryAllocatedForCollectionIndexCache(cost);
 }
 
 inline const Vector<Element*>* CollectionNamedElementCache::find(const StringToElementsMap& map, const AtomicString& key) const
index 3833748..e3e2718 100644 (file)
@@ -88,7 +88,9 @@ void HTMLImageLoader::imageChanged(CachedImage* cachedImage, const IntRect*)
         if (!element().inDocument()) {
             JSC::VM& vm = JSDOMWindowBase::commonVM();
             JSC::JSLockHolder lock(vm);
-            vm.heap.reportExtraMemoryCost(cachedImage->encodedSize());
+            // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+            // https://bugs.webkit.org/show_bug.cgi?id=142593
+            vm.heap.reportExtraMemoryAllocated(cachedImage->encodedSize());
         }
     }
 
index d43c18b..f4ff99a 100644 (file)
@@ -636,8 +636,11 @@ void HTMLMediaElement::removedFrom(ContainerNode& insertionPoint)
             size_t extraMemoryCostDelta = extraMemoryCost - m_reportedExtraMemoryCost;
             m_reportedExtraMemoryCost = extraMemoryCost;
 
-            if (extraMemoryCostDelta > 0)
-                vm.heap.reportExtraMemoryCost(extraMemoryCostDelta);
+            if (extraMemoryCostDelta > 0) {
+                // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+                // https://bugs.webkit.org/show_bug.cgi?id=142593
+                vm.heap.reportExtraMemoryAllocated(extraMemoryCostDelta);
+            }
         }
     }
 
index c096cb1..4e6f3fe 100644 (file)
@@ -913,7 +913,9 @@ void XMLHttpRequest::dropProtection()
     // report the extra cost at that point.
     JSC::VM& vm = scriptExecutionContext()->vm();
     JSC::JSLockHolder lock(vm);
-    vm.heap.reportExtraMemoryCost(m_responseBuilder.length() * 2);
+    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
+    // https://bugs.webkit.org/show_bug.cgi?id=142593
+    vm.heap.reportExtraMemoryAllocated(m_responseBuilder.length() * 2);
 
     unsetPendingActivity(this);
 }