<!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>[184407] trunk/Source/JavaScriptCore</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/184407">184407</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-05-15 13:02:26 -0700 (Fri, 15 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>JSArray::setLength() should reallocate instead of zero-filling if the reallocation would be small enough.
https://bugs.webkit.org/show_bug.cgi?id=144622
Reviewed by Geoffrey Garen.
When setting the array to a new length that is shorter, we now check if it is worth
just making a new butterfly instead of clearing out the slots in the old butterfly
that resides beyond the new length. If so, we will make a new butterfly instead.
There is no perf differences in the benchmark results. However, this does benefit
the perf of pathological cases where we need to shorten the length of a very large
array, as is the case in tests/mozilla/js1_5/Array/regress-101964.js. With this
patch, we can expect that test to complete in a short time again.
* runtime/JSArray.cpp:
(JSC::JSArray::setLength):
* runtime/JSObject.cpp:
(JSC::JSObject::reallocateAndShrinkButterfly):
- makes a new butterfly with a new shorter length.
* runtime/JSObject.h:
* tests/mozilla/js1_5/Array/regress-101964.js:
- Undo this test change since this patch will prevent us from spending a lot of time
clearing a large butterfly.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSArraycpp">trunk/Source/JavaScriptCore/runtime/JSArray.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjectcpp">trunk/Source/JavaScriptCore/runtime/JSObject.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjecth">trunk/Source/JavaScriptCore/runtime/JSObject.h</a></li>
<li><a href="#trunkSourceJavaScriptCoretestsmozillajs1_5Arrayregress101964js">trunk/Source/JavaScriptCore/tests/mozilla/js1_5/Array/regress-101964.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (184406 => 184407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-05-15 20:02:26 UTC (rev 184407)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2015-05-15 Mark Lam <mark.lam@apple.com>
+
+ JSArray::setLength() should reallocate instead of zero-filling if the reallocation would be small enough.
+ https://bugs.webkit.org/show_bug.cgi?id=144622
+
+ Reviewed by Geoffrey Garen.
+
+ When setting the array to a new length that is shorter, we now check if it is worth
+ just making a new butterfly instead of clearing out the slots in the old butterfly
+ that resides beyond the new length. If so, we will make a new butterfly instead.
+
+ There is no perf differences in the benchmark results. However, this does benefit
+ the perf of pathological cases where we need to shorten the length of a very large
+ array, as is the case in tests/mozilla/js1_5/Array/regress-101964.js. With this
+ patch, we can expect that test to complete in a short time again.
+
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::setLength):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::reallocateAndShrinkButterfly):
+ - makes a new butterfly with a new shorter length.
+ * runtime/JSObject.h:
+ * tests/mozilla/js1_5/Array/regress-101964.js:
+ - Undo this test change since this patch will prevent us from spending a lot of time
+ clearing a large butterfly.
+
</ins><span class="cx"> 2015-05-15 Basile Clement <basile_clement@apple.com>
</span><span class="cx">
</span><span class="cx"> DFGLICMPhase shouldn't create NodeOrigins with forExit but without semantic
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSArraycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSArray.cpp (184406 => 184407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSArray.cpp 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/JavaScriptCore/runtime/JSArray.cpp 2015-05-15 20:02:26 UTC (rev 184407)
</span><span class="lines">@@ -404,7 +404,7 @@
</span><span class="cx"> case ArrayWithUndecided:
</span><span class="cx"> case ArrayWithInt32:
</span><span class="cx"> case ArrayWithDouble:
</span><del>- case ArrayWithContiguous:
</del><ins>+ case ArrayWithContiguous: {
</ins><span class="cx"> if (newLength == m_butterfly->publicLength())
</span><span class="cx"> return true;
</span><span class="cx"> if (newLength >= MAX_ARRAY_INDEX // This case ensures that we can do fast push.
</span><span class="lines">@@ -418,6 +418,14 @@
</span><span class="cx"> ensureLength(exec->vm(), newLength);
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><ins>+
+ unsigned lengthToClear = m_butterfly->publicLength() - newLength;
+ unsigned costToAllocateNewButterfly = 64; // a heuristic.
+ if (lengthToClear > newLength && lengthToClear > costToAllocateNewButterfly) {
+ reallocateAndShrinkButterfly(exec->vm(), newLength);
+ return true;
+ }
+
</ins><span class="cx"> if (indexingType() == ArrayWithDouble) {
</span><span class="cx"> for (unsigned i = m_butterfly->publicLength(); i-- > newLength;)
</span><span class="cx"> m_butterfly->contiguousDouble()[i] = PNaN;
</span><span class="lines">@@ -427,6 +435,7 @@
</span><span class="cx"> }
</span><span class="cx"> m_butterfly->setPublicLength(newLength);
</span><span class="cx"> return true;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> case ArrayWithArrayStorage:
</span><span class="cx"> case ArrayWithSlowPutArrayStorage:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.cpp (184406 => 184407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.cpp 2015-05-15 20:02:26 UTC (rev 184407)
</span><span class="lines">@@ -2457,6 +2457,21 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void JSObject::reallocateAndShrinkButterfly(VM& vm, unsigned length)
+{
+ ASSERT(length < MAX_ARRAY_INDEX);
+ ASSERT(length < MAX_STORAGE_VECTOR_LENGTH);
+ ASSERT(hasContiguous(indexingType()) || hasInt32(indexingType()) || hasDouble(indexingType()) || hasUndecided(indexingType()));
+ ASSERT(m_butterfly->vectorLength() > length);
+ ASSERT(!m_butterfly->indexingHeader()->preCapacity(structure()));
+
+ DeferGC deferGC(vm.heap);
+ Butterfly* newButterfly = m_butterfly->resizeArray(vm, this, structure(), 0, ArrayStorage::sizeFor(length));
+ m_butterfly.set(vm, this, newButterfly);
+ m_butterfly->setVectorLength(length);
+ m_butterfly->setPublicLength(length);
+}
+
</ins><span class="cx"> Butterfly* JSObject::growOutOfLineStorage(VM& vm, size_t oldSize, size_t newSize)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(newSize > oldSize);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjecth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.h (184406 => 184407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.h 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.h 2015-05-15 20:02:26 UTC (rev 184407)
</span><span class="lines">@@ -827,6 +827,10 @@
</span><span class="cx"> m_butterfly->setPublicLength(length);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Call this if you want to shrink the butterfly backing store, and you're
+ // sure that the array is contiguous.
+ void reallocateAndShrinkButterfly(VM&, unsigned length);
+
</ins><span class="cx"> template<IndexingType indexingShape>
</span><span class="cx"> unsigned countElements(Butterfly*);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsmozillajs1_5Arrayregress101964js"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/tests/mozilla/js1_5/Array/regress-101964.js (184406 => 184407)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/mozilla/js1_5/Array/regress-101964.js 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/JavaScriptCore/tests/mozilla/js1_5/Array/regress-101964.js 2015-05-15 20:02:26 UTC (rev 184407)
</span><span class="lines">@@ -31,7 +31,7 @@
</span><span class="cx"> var summary = 'Performance: truncating even very large arrays should be fast!';
</span><span class="cx"> var BIG = 10000000;
</span><span class="cx"> var LITTLE = 10;
</span><del>-var FAST = 10000; // This used to test that array truncation should be 50 ms or less. We've changed it because we don't care how long it takes. We just try to make sure it doesn't take *ridiculously* long and we want it to run to completion correctly.
</del><ins>+var FAST = 50; // array truncation should be 50 ms or less to pass the test
</ins><span class="cx"> var MSG_FAST = 'Truncation took less than ' + FAST + ' ms';
</span><span class="cx"> var MSG_SLOW = 'Truncation took ';
</span><span class="cx"> var MSG_MS = ' ms';
</span></span></pre>
</div>
</div>
</body>
</html>