Many users of Heap::reportExtraMemory* are wrong, causing lots of memory growth
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Mar 2015 22:32:19 +0000 (22:32 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Mar 2015 22:32:19 +0000 (22:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142593

Reviewed by Andreas Kling.

Adopt deprecatedReportExtraMemory as a short-term fix for runaway
memory growth in these cases where we have not adopted
reportExtraMemoryVisited.

Long-term, we should use reportExtraMemoryAllocated+reportExtraMemoryVisited.
That's tracked by https://bugs.webkit.org/show_bug.cgi?id=142595.

Source/JavaScriptCore:

* API/JSBase.cpp:
(JSReportExtraMemoryCost):
* runtime/SparseArrayValueMap.cpp:
(JSC::SparseArrayValueMap::add):

Source/WebCore:

Using IOSDebug, I can see that the canvas stress test @ http://jsfiddle.net/fvyw4ba0/,
which used to keep > 1000 1MB NonVolatile GPU allocations live, now keeps about 10 live.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::reportExtraMemoryAllocated):
* bindings/js/JSDocumentCustom.cpp:
(WebCore::toJS):
* bindings/js/JSImageDataCustom.cpp:
(WebCore::toJS):
* bindings/js/JSNodeListCustom.cpp:
(WebCore::createWrapper):
* dom/CollectionIndexCache.cpp:
(WebCore::reportExtraMemoryAllocatedForCollectionIndexCache):
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::createImageBuffer):
* 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@181411 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/JavaScriptCore/API/JSBase.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Source/WebCore/bindings/js/JSDocumentCustom.cpp
Source/WebCore/bindings/js/JSImageDataCustom.cpp
Source/WebCore/bindings/js/JSNodeListCustom.cpp
Source/WebCore/dom/CollectionIndexCache.cpp
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/HTMLImageLoader.cpp
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/xml/XMLHttpRequest.cpp

index 7fc0f94..0325fec 100644 (file)
@@ -140,9 +140,7 @@ void JSReportExtraMemoryCost(JSContextRef ctx, size_t size)
     ExecState* exec = toJS(ctx);
     JSLockHolder locker(exec);
 
-    // FIXME: switch to deprecatedReportExtraMemory.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    exec->vm().heap.reportExtraMemoryAllocated(size);
+    exec->vm().heap.deprecatedReportExtraMemory(size);
 }
 
 extern "C" JS_EXPORT void JSSynchronousGarbageCollectForDebugging(JSContextRef);
index 095cb04..cd8aa1a 100644 (file)
@@ -1,5 +1,24 @@
 2015-03-11  Geoffrey Garen  <ggaren@apple.com>
 
+        Many users of Heap::reportExtraMemory* are wrong, causing lots of memory growth
+        https://bugs.webkit.org/show_bug.cgi?id=142593
+
+        Reviewed by Andreas Kling.
+
+        Adopt deprecatedReportExtraMemory as a short-term fix for runaway
+        memory growth in these cases where we have not adopted
+        reportExtraMemoryVisited.
+
+        Long-term, we should use reportExtraMemoryAllocated+reportExtraMemoryVisited.
+        That's tracked by https://bugs.webkit.org/show_bug.cgi?id=142595.
+
+        * API/JSBase.cpp:
+        (JSReportExtraMemoryCost):
+        * runtime/SparseArrayValueMap.cpp:
+        (JSC::SparseArrayValueMap::add):
+
+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
 
index 7c784ff..62d34ba 100644 (file)
@@ -80,9 +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) {
-        // 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>)));
+        // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+        // https://bugs.webkit.org/show_bug.cgi?id=142595
+        Heap::heap(array)->deprecatedReportExtraMemory((capacity - m_reportedCapacity) * (sizeof(unsigned) + sizeof(WriteBarrier<Unknown>)));
         m_reportedCapacity = capacity;
     }
     return result;
index 0e8e697..920237a 100644 (file)
@@ -1,3 +1,39 @@
+2015-03-11  Geoffrey Garen  <ggaren@apple.com>
+
+        Many users of Heap::reportExtraMemory* are wrong, causing lots of memory growth
+        https://bugs.webkit.org/show_bug.cgi?id=142593
+
+        Reviewed by Andreas Kling.
+
+        Adopt deprecatedReportExtraMemory as a short-term fix for runaway
+        memory growth in these cases where we have not adopted
+        reportExtraMemoryVisited.
+
+        Long-term, we should use reportExtraMemoryAllocated+reportExtraMemoryVisited.
+        That's tracked by https://bugs.webkit.org/show_bug.cgi?id=142595.
+
+        Using IOSDebug, I can see that the canvas stress test @ http://jsfiddle.net/fvyw4ba0/,
+        which used to keep > 1000 1MB NonVolatile GPU allocations live, now keeps about 10 live.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::reportExtraMemoryAllocated):
+        * bindings/js/JSDocumentCustom.cpp:
+        (WebCore::toJS):
+        * bindings/js/JSImageDataCustom.cpp:
+        (WebCore::toJS):
+        * bindings/js/JSNodeListCustom.cpp:
+        (WebCore::createWrapper):
+        * dom/CollectionIndexCache.cpp:
+        (WebCore::reportExtraMemoryAllocatedForCollectionIndexCache):
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::createImageBuffer):
+        * html/HTMLImageLoader.cpp:
+        (WebCore::HTMLImageLoader::imageChanged):
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::dropProtection):
+
 2015-03-10  Andy Estes  <aestes@apple.com>
 
         REGRESSION (r180985): contentfiltering/block-after-add-data.html crashes with GuardMalloc
index 3f16298..70a90b7 100644 (file)
@@ -1999,9 +1999,9 @@ void SourceBuffer::reportExtraMemoryAllocated()
 
     JSC::JSLockHolder lock(scriptExecutionContext()->vm());
     if (extraMemoryCostDelta > 0) {
-        // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-        // https://bugs.webkit.org/show_bug.cgi?id=142593
-        scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(extraMemoryCostDelta);
+        // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+        // https://bugs.webkit.org/show_bug.cgi?id=142595
+        scriptExecutionContext()->vm().heap.deprecatedReportExtraMemory(extraMemoryCostDelta);
     }
 }
 
index cd19a3d..8d1e894 100644 (file)
@@ -109,9 +109,9 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, Document* documen
         for (Node* n = document; n; n = NodeTraversal::next(*n))
             nodeCount++;
         
-        // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-        // https://bugs.webkit.org/show_bug.cgi?id=142593
-        exec->heap()->reportExtraMemoryAllocated(nodeCount * sizeof(Node));
+        // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+        // https://bugs.webkit.org/show_bug.cgi?id=142595
+        exec->heap()->deprecatedReportExtraMemory(nodeCount * sizeof(Node));
     }
 
     return wrapper;
index aa8766d..05c0cd9 100644 (file)
@@ -47,9 +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);
-    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    exec->heap()->reportExtraMemoryAllocated(imageData->data()->length());
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    exec->heap()->deprecatedReportExtraMemory(imageData->data()->length());
     
     return wrapper;
 }
index 9887dfc..ee230cb 100644 (file)
@@ -62,9 +62,9 @@ bool JSNodeList::getOwnPropertySlotDelegate(ExecState* exec, PropertyName proper
 
 JSC::JSValue createWrapper(JSDOMGlobalObject& globalObject, NodeList& nodeList)
 {
-    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    globalObject.vm().heap.reportExtraMemoryAllocated(nodeList.memoryCost());
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    globalObject.vm().heap.deprecatedReportExtraMemory(nodeList.memoryCost());
     return createNewWrapper<JSNodeList>(&globalObject, &nodeList);
 }
 
index cc81e0a..e15b591 100644 (file)
@@ -34,9 +34,9 @@ void reportExtraMemoryAllocatedForCollectionIndexCache(size_t cost)
 {
     JSC::VM& vm = JSDOMWindowBase::commonVM();
     JSC::JSLockHolder lock(vm);
-    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    vm.heap.reportExtraMemoryAllocated(cost);
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    vm.heap.deprecatedReportExtraMemory(cost);
 }
 
 }
index c2a08bf..12ef90d 100644 (file)
@@ -576,9 +576,9 @@ void HTMLCanvasElement::createImageBuffer() const
 
     JSC::JSLockHolder lock(scriptExecutionContext()->vm());
     size_t numBytes = 4 * m_imageBuffer->internalSize().width() * m_imageBuffer->internalSize().height();
-    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(numBytes);
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    scriptExecutionContext()->vm().heap.deprecatedReportExtraMemory(numBytes);
 
 #if USE(IOSURFACE_CANVAS_BACKING_STORE) || ENABLE(ACCELERATED_2D_CANVAS)
     if (m_context && m_context->is2d())
index e3e2718..fe50adc 100644 (file)
@@ -88,9 +88,9 @@ void HTMLImageLoader::imageChanged(CachedImage* cachedImage, const IntRect*)
         if (!element().inDocument()) {
             JSC::VM& vm = JSDOMWindowBase::commonVM();
             JSC::JSLockHolder lock(vm);
-            // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-            // https://bugs.webkit.org/show_bug.cgi?id=142593
-            vm.heap.reportExtraMemoryAllocated(cachedImage->encodedSize());
+            // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+            // https://bugs.webkit.org/show_bug.cgi?id=142595
+            vm.heap.deprecatedReportExtraMemory(cachedImage->encodedSize());
         }
     }
 
index f4ff99a..71dd636 100644 (file)
@@ -637,9 +637,9 @@ void HTMLMediaElement::removedFrom(ContainerNode& insertionPoint)
             m_reportedExtraMemoryCost = extraMemoryCost;
 
             if (extraMemoryCostDelta > 0) {
-                // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-                // https://bugs.webkit.org/show_bug.cgi?id=142593
-                vm.heap.reportExtraMemoryAllocated(extraMemoryCostDelta);
+                // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+                // https://bugs.webkit.org/show_bug.cgi?id=142595
+                vm.heap.deprecatedReportExtraMemory(extraMemoryCostDelta);
             }
         }
     }
index 4e6f3fe..51b69ac 100644 (file)
@@ -913,9 +913,9 @@ void XMLHttpRequest::dropProtection()
     // report the extra cost at that point.
     JSC::VM& vm = scriptExecutionContext()->vm();
     JSC::JSLockHolder lock(vm);
-    // FIXME: Switch to deprecatedReportExtraMemory, or adopt reportExtraMemoryVisited.
-    // https://bugs.webkit.org/show_bug.cgi?id=142593
-    vm.heap.reportExtraMemoryAllocated(m_responseBuilder.length() * 2);
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    vm.heap.deprecatedReportExtraMemory(m_responseBuilder.length() * 2);
 
     unsetPendingActivity(this);
 }