Diff
Modified: branches/safari-537-branch/LayoutTests/ChangeLog (153798 => 153799)
--- branches/safari-537-branch/LayoutTests/ChangeLog 2013-08-07 21:13:41 UTC (rev 153798)
+++ branches/safari-537-branch/LayoutTests/ChangeLog 2013-08-07 21:27:34 UTC (rev 153799)
@@ -1,3 +1,20 @@
+2013-08-07 Lucas Forschler <[email protected]>
+
+ Merge r153374
+
+ 2013-07-19 Mark Hahnenberg <[email protected]>
+
+ Setting a large numeric property on an object causes it to allocate a huge backing store
+ https://bugs.webkit.org/show_bug.cgi?id=118914
+
+ Reviewed by Geoffrey Garen.
+
+ Added new regression test for put-by-val-ing to a blank indexing type with a large index.
+ This fix is ~800x speedup on this regression test :-o
+
+ * fast/js/regress/put-by-val-large-index-blank-indexing-type.html: Added.
+ * fast/js/regress/script-tests/put-by-val-large-index-blank-indexing-type.js: Added.
+
2013-08-05 Lucas Forschler <[email protected]>
Merge r153725
Copied: branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type-expected.txt (from rev 153374, trunk/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type-expected.txt) (0 => 153799)
--- branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type-expected.txt (rev 0)
+++ branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type-expected.txt 2013-08-07 21:27:34 UTC (rev 153799)
@@ -0,0 +1,10 @@
+JSRegress/put-by-val-large-index-blank-indexing-type
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Copied: branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type.html (from rev 153374, trunk/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type.html) (0 => 153799)
--- branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type.html (rev 0)
+++ branches/safari-537-branch/LayoutTests/fast/js/regress/put-by-val-large-index-blank-indexing-type.html 2013-08-07 21:27:34 UTC (rev 153799)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>
Copied: branches/safari-537-branch/LayoutTests/fast/js/regress/script-tests/put-by-val-large-index-blank-indexing-type.js (from rev 153374, trunk/LayoutTests/fast/js/regress/script-tests/put-by-val-large-index-blank-indexing-type.js) (0 => 153799)
--- branches/safari-537-branch/LayoutTests/fast/js/regress/script-tests/put-by-val-large-index-blank-indexing-type.js (rev 0)
+++ branches/safari-537-branch/LayoutTests/fast/js/regress/script-tests/put-by-val-large-index-blank-indexing-type.js 2013-08-07 21:27:34 UTC (rev 153799)
@@ -0,0 +1,16 @@
+var global = [];
+
+function foo(o, id) {
+ o[id] = 42;
+}
+
+var sum = 0;
+for (var i = 0; i < 20000; i++) {
+ var o = {};
+ foo(o, i);
+ sum += o[i];
+ global.push(o);
+}
+
+if (sum != 42 * 20000)
+ throw "Error!"
Modified: branches/safari-537-branch/Source/_javascript_Core/ChangeLog (153798 => 153799)
--- branches/safari-537-branch/Source/_javascript_Core/ChangeLog 2013-08-07 21:13:41 UTC (rev 153798)
+++ branches/safari-537-branch/Source/_javascript_Core/ChangeLog 2013-08-07 21:27:34 UTC (rev 153799)
@@ -1,3 +1,50 @@
+2013-08-07 Lucas Forschler <[email protected]>
+
+ Merge r153374
+
+ 2013-07-19 Mark Hahnenberg <[email protected]>
+
+ Setting a large numeric property on an object causes it to allocate a huge backing store
+ https://bugs.webkit.org/show_bug.cgi?id=118914
+
+ Reviewed by Geoffrey Garen.
+
+ There are two distinct actions that we're trying to optimize for:
+
+ new Array(100000);
+
+ and:
+
+ a = [];
+ a[100000] = 42;
+
+ In the first case, the programmer has indicated that they expect this Array to be very big,
+ so they should get a contiguous array up until some threshold, above which we perform density
+ calculations to see if it is indeed dense enough to warrant being contiguous.
+
+ In the second case, the programmer hasn't indicated anything about the size of the Array, so
+ we should be more conservative and assume it should be sparse until we've proven otherwise.
+
+ Currently both of those cases are handled by MIN_SPARSE_ARRAY_INDEX. We should distinguish
+ between them for the purposes of not over-allocating large backing stores like we see on
+ http://www.peekanalytics.com/burgerjoints/
+
+ The way that we'll do this is to keep the MIN_SPARSE_ARRAY_INDEX for the first case, and
+ introduce a new heuristic for the second case. If we are putting to an index above a certain
+ threshold (say, 1000) and it is beyond the length of the array, then we will use a sparse
+ map instead. So for example, in the second case above the empty array has a blank indexing
+ type and a length of 0. We put-by-val to an index > 1000 and > a.length, so we'll use a sparse map.
+
+ This fix is ~800x speedup on the accompanying regression test :-o
+
+ * runtime/ArrayConventions.h:
+ (JSC::indexIsSufficientlyBeyondLengthForSparseMap):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::putByIndexBeyondVectorLengthWithoutAttributes):
+ (JSC::JSObject::putByIndexBeyondVectorLengthWithArrayStorage):
+ (JSC::JSObject::putByIndexBeyondVectorLength):
+ (JSC::JSObject::putDirectIndexBeyondVectorLengthWithArrayStorage):
+
2013-08-01 Lucas Forschler <[email protected]>
Merge r153615
Modified: branches/safari-537-branch/Source/_javascript_Core/runtime/ArrayConventions.h (153798 => 153799)
--- branches/safari-537-branch/Source/_javascript_Core/runtime/ArrayConventions.h 2013-08-07 21:13:41 UTC (rev 153798)
+++ branches/safari-537-branch/Source/_javascript_Core/runtime/ArrayConventions.h 2013-08-07 21:27:34 UTC (rev 153799)
@@ -71,6 +71,8 @@
// is added.
#define FIRST_VECTOR_GROW 4U
+#define MIN_BEYOND_LENGTH_SPARSE_INDEX 1000
+
// Our policy for when to use a vector and when to use a sparse map.
// For all array indices under MIN_SPARSE_ARRAY_INDEX, we always use a vector.
// When indices greater than MIN_SPARSE_ARRAY_INDEX are involved, we use a vector
@@ -82,6 +84,11 @@
return length / minDensityMultiplier <= numValues;
}
+inline bool indexIsSufficientlyBeyondLengthForSparseMap(unsigned i, unsigned length)
+{
+ return i >= MIN_BEYOND_LENGTH_SPARSE_INDEX && i > length;
+}
+
inline IndexingHeader indexingHeaderForArray(unsigned length, unsigned vectorLength)
{
IndexingHeader result;
Modified: branches/safari-537-branch/Source/_javascript_Core/runtime/JSObject.cpp (153798 => 153799)
--- branches/safari-537-branch/Source/_javascript_Core/runtime/JSObject.cpp 2013-08-07 21:13:41 UTC (rev 153798)
+++ branches/safari-537-branch/Source/_javascript_Core/runtime/JSObject.cpp 2013-08-07 21:27:34 UTC (rev 153799)
@@ -1872,7 +1872,8 @@
if (i >= MAX_ARRAY_INDEX - 1
|| (i >= MIN_SPARSE_ARRAY_INDEX
- && !isDenseEnoughForVector(i, countElements<indexingShape>(m_butterfly)))) {
+ && !isDenseEnoughForVector(i, countElements<indexingShape>(m_butterfly)))
+ || indexIsSufficientlyBeyondLengthForSparseMap(i, m_butterfly->vectorLength())) {
ASSERT(i <= MAX_ARRAY_INDEX);
ensureArrayStorageSlow(vm);
SparseArrayValueMap* map = allocateSparseIndexMap(vm);
@@ -1920,7 +1921,7 @@
// First, handle cases where we don't currently have a sparse map.
if (LIKELY(!map)) {
- // If the array is not extensible, we should have entered dictionary mode, and created the spare map.
+ // If the array is not extensible, we should have entered dictionary mode, and created the sparse map.
ASSERT(isExtensible());
// Update m_length if necessary.
@@ -1928,7 +1929,9 @@
storage->setLength(i + 1);
// Check that it is sensible to still be using a vector, and then try to grow the vector.
- if (LIKELY((isDenseEnoughForVector(i, storage->m_numValuesInVector)) && increaseVectorLength(vm, i + 1))) {
+ if (LIKELY(!indexIsSufficientlyBeyondLengthForSparseMap(i, storage->vectorLength())
+ && isDenseEnoughForVector(i, storage->m_numValuesInVector)
+ && increaseVectorLength(vm, i + 1))) {
// success! - reread m_storage since it has likely been reallocated, and store to the vector.
storage = arrayStorage();
storage->m_vector[i].set(vm, this, value);
@@ -1995,7 +1998,7 @@
ensureArrayStorageExistsAndEnterDictionaryIndexingMode(vm));
break;
}
- if (i >= MIN_SPARSE_ARRAY_INDEX) {
+ if (indexIsSufficientlyBeyondLengthForSparseMap(i, 0) || i >= MIN_SPARSE_ARRAY_INDEX) {
putByIndexBeyondVectorLengthWithArrayStorage(
exec, i, value, shouldThrow, createArrayStorage(vm, 0, 0));
break;
@@ -2075,7 +2078,8 @@
if (LIKELY(
!attributes
&& (isDenseEnoughForVector(i, storage->m_numValuesInVector))
- && increaseVectorLength(vm, i + 1))) {
+ && increaseVectorLength(vm, i + 1)
+ && !indexIsSufficientlyBeyondLengthForSparseMap(i, storage->vectorLength()))) {
// success! - reread m_storage since it has likely been reallocated, and store to the vector.
storage = arrayStorage();
storage->m_vector[i].set(vm, this, value);
