<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[197543] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/197543">197543</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-03-03 19:19:11 -0800 (Thu, 03 Mar 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>JSArrayBuffers should be collected less aggressively
https://bugs.webkit.org/show_bug.cgi?id=154982
Reviewed by Geoffrey Garen.
Source/JavaScriptCore:
We are currently too aggressive in our collection of ArrayBuffer wrappers.
There are three cases where we need to avoid collecting ArrayBuffer wrappers.
1. If the wrapper has custom properties.
2. If the wrapper is a subclass of ArrayBuffer.
3. If the wrapper is in a WeakMap/WeakSet.
Currently, we only pass the first case in WebCore and none in the jsc CLI.
This patch removes some optimizations that cause us to collect when we
should not. Namely, always skipping the object unless it has custom
properties. Additionally, in the case of subclassing, we also need a way
for custom JSArrayBuffer objects to register themselves as the wrapper for
an ArrayBuffer class.
Finally, this patch fixes an issue where views would not mark their ArrayBuffer
as an opaque root. This patch also moves an associated ASSERT that the
ArrayBuffer held by a view is not null in JSGenericTypedArrayView::visitChildren
into JSArrayBufferView::visitChildren, where we add the opaque root.
* runtime/JSArrayBuffer.cpp:
(JSC::JSArrayBuffer::finishCreation):
(JSC::JSArrayBuffer::create):
(JSC::JSArrayBuffer::createWithoutWrapping):
* runtime/JSArrayBuffer.h:
* runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::visitChildren):
* runtime/JSArrayBufferView.h:
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::visitChildren): Deleted.
* runtime/SimpleTypedArrayController.cpp:
(JSC::SimpleTypedArrayController::toJS):
(JSC::SimpleTypedArrayController::registerWrapper):
(JSC::SimpleTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots):
(JSC::SimpleTypedArrayController::JSArrayBufferOwner::finalize):
* runtime/SimpleTypedArrayController.h:
* runtime/TypedArrayController.h:
Source/WebCore:
We are currently too aggressive in our collection of ArrayBuffer wrappers.
There are three cases where we need to avoid collecting ArrayBuffer wrappers.
1. If the wrapper has custom properties.
2. If the wrapper is a subclass of ArrayBuffer.
3. If the wrapper is in a WeakMap/WeakSet.
Currently, we only pass the first case in WebCore and none in the jsc CLI.
This patch removes some optimizations that cause us to collect when we
should not. Namely, always skipping the object unless it has custom
properties. Additionally, in the case of subclassing, we also need a way
for custom JSArrayBuffer objects to register themselves as the wrapper for
an ArrayBuffer class.
Finally, this patch removes an unnecessary ref() and deref() of
ArrayBuffer objects during the creation/destruction of the wrapper.
Since an ArrayBuffer object's GC lifetime will be at least as long
as the lifetime of the wrapper we are creating for it we don't need
to ref() and deref() the ArrayBuffer object. This lifetime is
guaranteed because ArrayBuffer objects are both GCed and refcounted
and any new wrapper will visit the ArrayBuffer object as long as the
wrapper is still alive.
Test: js/arraybuffer-wrappers.html
* bindings/js/JSDOMBinding.h:
(WebCore::toJS):
* bindings/js/WebCoreTypedArrayController.cpp:
(WebCore::WebCoreTypedArrayController::registerWrapper):
(WebCore::WebCoreTypedArrayController::JSArrayBufferOwner::finalize):
(WebCore::WebCoreTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots): Deleted.
* bindings/js/WebCoreTypedArrayController.h:
LayoutTests:
* js/arraybuffer-wrappers-expected.txt: Added.
* js/arraybuffer-wrappers.html: Added.
* js/script-tests/arraybuffer-wrappers.js: Added.
(prototype.types.forEach):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArrayBuffercpp">trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArrayBufferh">trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArrayBufferViewcpp">trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArrayBufferViewh">trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSGenericTypedArrayViewInlinesh">trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeSimpleTypedArrayControllercpp">trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeSimpleTypedArrayControllerh">trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeTypedArrayControllerh">trunk/Source/JavaScriptCore/runtime/TypedArrayController.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSDOMBindingh">trunk/Source/WebCore/bindings/js/JSDOMBinding.h</a></li>
<li><a href="#trunkSourceWebCorebindingsjsWebCoreTypedArrayControllercpp">trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.cpp</a></li>
<li><a href="#trunkSourceWebCorebindingsjsWebCoreTypedArrayControllerh">trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsjsarraybufferwrappersexpectedtxt">trunk/LayoutTests/js/arraybuffer-wrappers-expected.txt</a></li>
<li><a href="#trunkLayoutTestsjsarraybufferwrappershtml">trunk/LayoutTests/js/arraybuffer-wrappers.html</a></li>
<li><a href="#trunkLayoutTestsjsscripttestsarraybufferwrappersjs">trunk/LayoutTests/js/script-tests/arraybuffer-wrappers.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/LayoutTests/ChangeLog 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-03-03 Keith Miller <keith_miller@apple.com>
+
+ JSArrayBuffers should be collected less aggressively
+ https://bugs.webkit.org/show_bug.cgi?id=154982
+
+ Reviewed by Geoffrey Garen.
+
+ * js/arraybuffer-wrappers-expected.txt: Added.
+ * js/arraybuffer-wrappers.html: Added.
+ * js/script-tests/arraybuffer-wrappers.js: Added.
+ (prototype.types.forEach):
+
</ins><span class="cx"> 2016-03-03 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Use larger tiles when possible to reduce per-tile painting overhead
</span></span></pre></div>
<a id="trunkLayoutTestsjsarraybufferwrappersexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/arraybuffer-wrappers-expected.txt (0 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/arraybuffer-wrappers-expected.txt (rev 0)
+++ trunk/LayoutTests/js/arraybuffer-wrappers-expected.txt 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -0,0 +1,44 @@
</span><ins>+Tests wrappers for ArrayBuffer objects are not GCed when they shouldn't be
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Test subclassing
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+PASS foo.buffer instanceof C is true
+
+Test properties
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+PASS foo.buffer.bar is 1
+
+Test WeakMap
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS ws.has(foo.buffer) is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsjsarraybufferwrappershtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/arraybuffer-wrappers.html (0 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/arraybuffer-wrappers.html (rev 0)
+++ trunk/LayoutTests/js/arraybuffer-wrappers.html 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/arraybuffer-wrappers.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsjsscripttestsarraybufferwrappersjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/js/script-tests/arraybuffer-wrappers.js (0 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/script-tests/arraybuffer-wrappers.js (rev 0)
+++ trunk/LayoutTests/js/script-tests/arraybuffer-wrappers.js 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -0,0 +1,47 @@
</span><ins>+description("Tests wrappers for ArrayBuffer objects are not GCed when they shouldn't be");
+
+let types = [Int8Array, Uint8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array, DataView];
+
+debug("Test subclassing");
+
+types.forEach(function(type) {
+ C = class extends ArrayBuffer { }
+
+ let b = new C(8);
+ foo = new type(b);
+
+ b = null;
+ gc();
+
+ shouldBeTrue("foo.buffer instanceof C");
+});
+
+debug("");
+debug("Test properties");
+
+types.forEach(function(type) {
+ b = new ArrayBuffer(8);
+ b.bar = 1;
+
+ foo = new Int32Array(b);
+ b = null;
+ gc();
+
+ shouldBe("foo.buffer.bar", "1");
+});
+
+debug("");
+debug("Test WeakMap");
+
+types.forEach(function(type) {
+ ws = new WeakSet();
+ b = new ArrayBuffer(8);
+
+ ws.add(b);
+ foo = new Int32Array(b);
+ b = null;
+
+ gc();
+
+ shouldBeTrue("ws.has(foo.buffer)");
+});
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -1,3 +1,46 @@
</span><ins>+2016-03-03 Keith Miller <keith_miller@apple.com>
+
+ JSArrayBuffers should be collected less aggressively
+ https://bugs.webkit.org/show_bug.cgi?id=154982
+
+ Reviewed by Geoffrey Garen.
+
+ We are currently too aggressive in our collection of ArrayBuffer wrappers.
+ There are three cases where we need to avoid collecting ArrayBuffer wrappers.
+ 1. If the wrapper has custom properties.
+ 2. If the wrapper is a subclass of ArrayBuffer.
+ 3. If the wrapper is in a WeakMap/WeakSet.
+
+ Currently, we only pass the first case in WebCore and none in the jsc CLI.
+ This patch removes some optimizations that cause us to collect when we
+ should not. Namely, always skipping the object unless it has custom
+ properties. Additionally, in the case of subclassing, we also need a way
+ for custom JSArrayBuffer objects to register themselves as the wrapper for
+ an ArrayBuffer class.
+
+ Finally, this patch fixes an issue where views would not mark their ArrayBuffer
+ as an opaque root. This patch also moves an associated ASSERT that the
+ ArrayBuffer held by a view is not null in JSGenericTypedArrayView::visitChildren
+ into JSArrayBufferView::visitChildren, where we add the opaque root.
+
+ * runtime/JSArrayBuffer.cpp:
+ (JSC::JSArrayBuffer::finishCreation):
+ (JSC::JSArrayBuffer::create):
+ (JSC::JSArrayBuffer::createWithoutWrapping):
+ * runtime/JSArrayBuffer.h:
+ * runtime/JSArrayBufferView.cpp:
+ (JSC::JSArrayBufferView::visitChildren):
+ * runtime/JSArrayBufferView.h:
+ * runtime/JSGenericTypedArrayViewInlines.h:
+ (JSC::JSGenericTypedArrayView<Adaptor>::visitChildren): Deleted.
+ * runtime/SimpleTypedArrayController.cpp:
+ (JSC::SimpleTypedArrayController::toJS):
+ (JSC::SimpleTypedArrayController::registerWrapper):
+ (JSC::SimpleTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots):
+ (JSC::SimpleTypedArrayController::JSArrayBufferOwner::finalize):
+ * runtime/SimpleTypedArrayController.h:
+ * runtime/TypedArrayController.h:
+
</ins><span class="cx"> 2016-03-03 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Octane/regexp's Exec function should benefit from array length accessor inlining
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArrayBuffercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.cpp (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.cpp 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.cpp 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -40,10 +40,11 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void JSArrayBuffer::finishCreation(VM& vm)
</del><ins>+void JSArrayBuffer::finishCreation(VM& vm, JSGlobalObject* globalObject)
</ins><span class="cx"> {
</span><span class="cx"> Base::finishCreation(vm);
</span><span class="cx"> vm.heap.addReference(this, m_impl);
</span><ins>+ vm.m_typedArrayController->registerWrapper(globalObject, m_impl, this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> JSArrayBuffer* JSArrayBuffer::create(
</span><span class="lines">@@ -53,7 +54,7 @@
</span><span class="cx"> JSArrayBuffer* result =
</span><span class="cx"> new (NotNull, allocateCell<JSArrayBuffer>(vm.heap))
</span><span class="cx"> JSArrayBuffer(vm, structure, buffer);
</span><del>- result->finishCreation(vm);
</del><ins>+ result->finishCreation(vm, structure->globalObject());
</ins><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArrayBufferh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/JSArrayBuffer.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -38,11 +38,12 @@
</span><span class="cx">
</span><span class="cx"> protected:
</span><span class="cx"> JSArrayBuffer(VM&, Structure*, PassRefPtr<ArrayBuffer>);
</span><del>- void finishCreation(VM&);
</del><ins>+ void finishCreation(VM&, JSGlobalObject*);
</ins><span class="cx">
</span><span class="cx"> public:
</span><ins>+ // This function will register the new wrapper with the vm's TypedArrayController.
</ins><span class="cx"> JS_EXPORT_PRIVATE static JSArrayBuffer* create(VM&, Structure*, PassRefPtr<ArrayBuffer>);
</span><del>-
</del><ins>+
</ins><span class="cx"> ArrayBuffer* impl() const { return m_impl; }
</span><span class="cx">
</span><span class="cx"> static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArrayBufferViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.cpp 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -151,6 +151,19 @@
</span><span class="cx"> return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void JSArrayBufferView::visitChildren(JSCell* cell, SlotVisitor& visitor)
+{
+ JSArrayBufferView* thisObject = jsCast<JSArrayBufferView*>(cell);
+
+ if (thisObject->hasArrayBuffer()) {
+ ArrayBuffer* buffer = thisObject->buffer();
+ RELEASE_ASSERT(buffer);
+ visitor.addOpaqueRoot(buffer);
+ }
+
+ Base::visitChildren(thisObject, visitor);
+}
+
</ins><span class="cx"> void JSArrayBufferView::put(
</span><span class="cx"> JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value,
</span><span class="cx"> PutPropertySlot& slot)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArrayBufferViewh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -150,6 +150,8 @@
</span><span class="cx"> static void put(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
</span><span class="cx"> static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
</span><span class="cx"> static bool deleteProperty(JSCell*, ExecState*, PropertyName);
</span><ins>+
+ static void visitChildren(JSCell*, SlotVisitor&);
</ins><span class="cx">
</span><span class="cx"> static void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSGenericTypedArrayViewInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -436,7 +436,6 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case WastefulTypedArray:
</span><del>- RELEASE_ASSERT(thisObject->existingBufferInButterfly());
</del><span class="cx"> break;
</span><span class="cx">
</span><span class="cx"> case DataViewMode:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeSimpleTypedArrayControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.cpp (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.cpp 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.cpp 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -40,12 +40,26 @@
</span><span class="cx"> {
</span><span class="cx"> if (JSArrayBuffer* buffer = native->m_wrapper.get())
</span><span class="cx"> return buffer;
</span><del>-
</del><ins>+
+ // The JSArrayBuffer::create function will register the wrapper in finishCreation.
</ins><span class="cx"> JSArrayBuffer* result = JSArrayBuffer::create(
</span><span class="cx"> exec->vm(), globalObject->arrayBufferStructure(), native);
</span><del>- native->m_wrapper = Weak<JSArrayBuffer>(result);
</del><span class="cx"> return result;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void SimpleTypedArrayController::registerWrapper(JSGlobalObject*, ArrayBuffer* native, JSArrayBuffer* wrapper)
+{
+ ASSERT(!native->m_wrapper);
+ native->m_wrapper = Weak<JSArrayBuffer>(wrapper, &m_owner);
+}
+
+bool SimpleTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, JSC::SlotVisitor& visitor)
+{
+ auto& wrapper = *JSC::jsCast<JSC::JSArrayBuffer*>(handle.slot()->asCell());
+ return visitor.containsOpaqueRoot(wrapper.impl());
+}
+
+void SimpleTypedArrayController::JSArrayBufferOwner::finalize(JSC::Handle<JSC::Unknown>, void*) { }
+
</ins><span class="cx"> } // namespace JSC
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeSimpleTypedArrayControllerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/SimpleTypedArrayController.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -28,6 +28,7 @@
</span><span class="cx">
</span><span class="cx"> #include "Handle.h"
</span><span class="cx"> #include "TypedArrayController.h"
</span><ins>+#include "WeakHandleOwner.h"
</ins><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span><span class="lines">@@ -38,6 +39,10 @@
</span><span class="cx"> //
</span><span class="cx"> // - If the JSArrayBuffer is live, then the ArrayBuffer stays alive.
</span><span class="cx"> //
</span><ins>+// - If there is a JSArrayBufferView that is holding an ArrayBuffer
+// then any existing wrapper for that ArrayBuffer will be kept
+// alive.
+//
</ins><span class="cx"> // - If you ask an ArrayBuffer for a JSArrayBuffer after one had
</span><span class="cx"> // already been created and it didn't die, then you get the same
</span><span class="cx"> // one.
</span><span class="lines">@@ -48,6 +53,16 @@
</span><span class="cx"> virtual ~SimpleTypedArrayController();
</span><span class="cx">
</span><span class="cx"> virtual JSArrayBuffer* toJS(ExecState*, JSGlobalObject*, ArrayBuffer*) override;
</span><ins>+ virtual void registerWrapper(JSGlobalObject*, ArrayBuffer*, JSArrayBuffer*) override;
+
+private:
+ class JSArrayBufferOwner : public WeakHandleOwner {
+ public:
+ virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, SlotVisitor&) override;
+ virtual void finalize(JSC::Handle<JSC::Unknown>, void* context) override;
+ };
+
+ JSArrayBufferOwner m_owner;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeTypedArrayControllerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/TypedArrayController.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/TypedArrayController.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/JavaScriptCore/runtime/TypedArrayController.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -41,6 +41,7 @@
</span><span class="cx"> JS_EXPORT_PRIVATE virtual ~TypedArrayController();
</span><span class="cx">
</span><span class="cx"> virtual JSArrayBuffer* toJS(ExecState*, JSGlobalObject*, ArrayBuffer*) = 0;
</span><ins>+ virtual void registerWrapper(JSGlobalObject*, ArrayBuffer*, JSArrayBuffer*) = 0;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/WebCore/ChangeLog 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2016-03-03 Keith Miller <keith_miller@apple.com>
+
+ JSArrayBuffers should be collected less aggressively
+ https://bugs.webkit.org/show_bug.cgi?id=154982
+
+ Reviewed by Geoffrey Garen.
+
+ We are currently too aggressive in our collection of ArrayBuffer wrappers.
+ There are three cases where we need to avoid collecting ArrayBuffer wrappers.
+ 1. If the wrapper has custom properties.
+ 2. If the wrapper is a subclass of ArrayBuffer.
+ 3. If the wrapper is in a WeakMap/WeakSet.
+
+ Currently, we only pass the first case in WebCore and none in the jsc CLI.
+ This patch removes some optimizations that cause us to collect when we
+ should not. Namely, always skipping the object unless it has custom
+ properties. Additionally, in the case of subclassing, we also need a way
+ for custom JSArrayBuffer objects to register themselves as the wrapper for
+ an ArrayBuffer class.
+
+ Finally, this patch removes an unnecessary ref() and deref() of
+ ArrayBuffer objects during the creation/destruction of the wrapper.
+ Since an ArrayBuffer object's GC lifetime will be at least as long
+ as the lifetime of the wrapper we are creating for it we don't need
+ to ref() and deref() the ArrayBuffer object. This lifetime is
+ guaranteed because ArrayBuffer objects are both GCed and refcounted
+ and any new wrapper will visit the ArrayBuffer object as long as the
+ wrapper is still alive.
+
+ Test: js/arraybuffer-wrappers.html
+
+ * bindings/js/JSDOMBinding.h:
+ (WebCore::toJS):
+ * bindings/js/WebCoreTypedArrayController.cpp:
+ (WebCore::WebCoreTypedArrayController::registerWrapper):
+ (WebCore::WebCoreTypedArrayController::JSArrayBufferOwner::finalize):
+ (WebCore::WebCoreTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots): Deleted.
+ * bindings/js/WebCoreTypedArrayController.h:
+
</ins><span class="cx"> 2016-03-03 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Use larger tiles when possible to reduce per-tile painting overhead
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSDOMBindingh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -410,9 +410,9 @@
</span><span class="cx"> return JSC::jsNull();
</span><span class="cx"> if (JSC::JSValue result = getExistingWrapper<JSC::JSArrayBuffer>(globalObject, buffer))
</span><span class="cx"> return result;
</span><del>- buffer->ref();
</del><ins>+
+ // The JSArrayBuffer::create function will register the wrapper in finishCreation.
</ins><span class="cx"> JSC::JSArrayBuffer* wrapper = JSC::JSArrayBuffer::create(exec->vm(), globalObject->arrayBufferStructure(), buffer);
</span><del>- cacheWrapper(globalObject->world(), buffer, wrapper);
</del><span class="cx"> return wrapper;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsWebCoreTypedArrayControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.cpp (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.cpp 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.cpp 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -47,20 +47,21 @@
</span><span class="cx"> return JSC::jsCast<JSC::JSArrayBuffer*>(WebCore::toJS(state, JSC::jsCast<JSDOMGlobalObject*>(globalObject), buffer));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void WebCoreTypedArrayController::registerWrapper(JSC::JSGlobalObject* globalObject, JSC::ArrayBuffer* native, JSC::JSArrayBuffer* wrapper)
+{
+ cacheWrapper(JSC::jsCast<JSDOMGlobalObject*>(globalObject)->world(), native, wrapper);
+}
+
</ins><span class="cx"> bool WebCoreTypedArrayController::JSArrayBufferOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, JSC::SlotVisitor& visitor)
</span><span class="cx"> {
</span><span class="cx"> auto& wrapper = *JSC::jsCast<JSC::JSArrayBuffer*>(handle.slot()->asCell());
</span><del>- if (!wrapper.hasCustomProperties())
- return false;
</del><span class="cx"> return visitor.containsOpaqueRoot(wrapper.impl());
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void WebCoreTypedArrayController::JSArrayBufferOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
</span><span class="cx"> {
</span><span class="cx"> auto& wrapper = *static_cast<JSC::JSArrayBuffer*>(handle.slot()->asCell());
</span><del>- auto& buffer = *wrapper.impl();
- uncacheWrapper(*static_cast<DOMWrapperWorld*>(context), &buffer, &wrapper);
- buffer.deref();
</del><ins>+ uncacheWrapper(*static_cast<DOMWrapperWorld*>(context), wrapper.impl(), &wrapper);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsWebCoreTypedArrayControllerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.h (197542 => 197543)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.h 2016-03-04 02:43:53 UTC (rev 197542)
+++ trunk/Source/WebCore/bindings/js/WebCoreTypedArrayController.h 2016-03-04 03:19:11 UTC (rev 197543)
</span><span class="lines">@@ -41,7 +41,8 @@
</span><span class="cx"> virtual ~WebCoreTypedArrayController();
</span><span class="cx">
</span><span class="cx"> virtual JSC::JSArrayBuffer* toJS(JSC::ExecState*, JSC::JSGlobalObject*, JSC::ArrayBuffer*) override;
</span><del>-
</del><ins>+ virtual void registerWrapper(JSC::JSGlobalObject*, ArrayBuffer*, JSC::JSArrayBuffer*) override;
+
</ins><span class="cx"> JSC::WeakHandleOwner* wrapperOwner() { return &m_owner; }
</span><span class="cx">
</span><span class="cx"> private:
</span></span></pre>
</div>
</div>
</body>
</html>