Title: [184407] trunk/Source/_javascript_Core
- Revision
- 184407
- Author
- [email protected]
- Date
- 2015-05-15 13:02:26 -0700 (Fri, 15 May 2015)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (184406 => 184407)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-15 20:02:26 UTC (rev 184407)
@@ -1,3 +1,29 @@
+2015-05-15 Mark Lam <[email protected]>
+
+ 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.
+
2015-05-15 Basile Clement <[email protected]>
DFGLICMPhase shouldn't create NodeOrigins with forExit but without semantic
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (184406 => 184407)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2015-05-15 20:02:26 UTC (rev 184407)
@@ -404,7 +404,7 @@
case ArrayWithUndecided:
case ArrayWithInt32:
case ArrayWithDouble:
- case ArrayWithContiguous:
+ case ArrayWithContiguous: {
if (newLength == m_butterfly->publicLength())
return true;
if (newLength >= MAX_ARRAY_INDEX // This case ensures that we can do fast push.
@@ -418,6 +418,14 @@
ensureLength(exec->vm(), newLength);
return true;
}
+
+ unsigned lengthToClear = m_butterfly->publicLength() - newLength;
+ unsigned costToAllocateNewButterfly = 64; // a heuristic.
+ if (lengthToClear > newLength && lengthToClear > costToAllocateNewButterfly) {
+ reallocateAndShrinkButterfly(exec->vm(), newLength);
+ return true;
+ }
+
if (indexingType() == ArrayWithDouble) {
for (unsigned i = m_butterfly->publicLength(); i-- > newLength;)
m_butterfly->contiguousDouble()[i] = PNaN;
@@ -427,6 +435,7 @@
}
m_butterfly->setPublicLength(newLength);
return true;
+ }
case ArrayWithArrayStorage:
case ArrayWithSlowPutArrayStorage:
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (184406 => 184407)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-05-15 20:02:26 UTC (rev 184407)
@@ -2457,6 +2457,21 @@
}
}
+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);
+}
+
Butterfly* JSObject::growOutOfLineStorage(VM& vm, size_t oldSize, size_t newSize)
{
ASSERT(newSize > oldSize);
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (184406 => 184407)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2015-05-15 20:02:26 UTC (rev 184407)
@@ -827,6 +827,10 @@
m_butterfly->setPublicLength(length);
}
+ // 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);
+
template<IndexingType indexingShape>
unsigned countElements(Butterfly*);
Modified: trunk/Source/_javascript_Core/tests/mozilla/js1_5/Array/regress-101964.js (184406 => 184407)
--- trunk/Source/_javascript_Core/tests/mozilla/js1_5/Array/regress-101964.js 2015-05-15 20:02:13 UTC (rev 184406)
+++ trunk/Source/_javascript_Core/tests/mozilla/js1_5/Array/regress-101964.js 2015-05-15 20:02:26 UTC (rev 184407)
@@ -31,7 +31,7 @@
var summary = 'Performance: truncating even very large arrays should be fast!';
var BIG = 10000000;
var LITTLE = 10;
-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.
+var FAST = 50; // array truncation should be 50 ms or less to pass the test
var MSG_FAST = 'Truncation took less than ' + FAST + ' ms';
var MSG_SLOW = 'Truncation took ';
var MSG_MS = ' ms';
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
