Ignore:
Timestamp:
Jul 26, 2013, 11:07:34 AM (12 years ago)
Author:
[email protected]
Message:

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.

Source/JavaScriptCore:

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):

LayoutTests:

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.
File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/runtime/JSObject.cpp

    r153223 r153374  
    18541854    if (i >= MAX_ARRAY_INDEX - 1
    18551855        || (i >= MIN_SPARSE_ARRAY_INDEX
    1856             && !isDenseEnoughForVector(i, countElements<indexingShape>(m_butterfly)))) {
     1856            && !isDenseEnoughForVector(i, countElements<indexingShape>(m_butterfly)))
     1857        || indexIsSufficientlyBeyondLengthForSparseMap(i, m_butterfly->vectorLength())) {
    18571858        ASSERT(i <= MAX_ARRAY_INDEX);
    18581859        ensureArrayStorageSlow(vm);
     
    19021903    // First, handle cases where we don't currently have a sparse map.
    19031904    if (LIKELY(!map)) {
    1904         // If the array is not extensible, we should have entered dictionary mode, and created the spare map.
     1905        // If the array is not extensible, we should have entered dictionary mode, and created the sparse map.
    19051906        ASSERT(isExtensible());
    19061907   
     
    19101911
    19111912        // Check that it is sensible to still be using a vector, and then try to grow the vector.
    1912         if (LIKELY((isDenseEnoughForVector(i, storage->m_numValuesInVector)) && increaseVectorLength(vm, i + 1))) {
     1913        if (LIKELY(!indexIsSufficientlyBeyondLengthForSparseMap(i, storage->vectorLength())
     1914            && isDenseEnoughForVector(i, storage->m_numValuesInVector)
     1915            && increaseVectorLength(vm, i + 1))) {
    19131916            // success! - reread m_storage since it has likely been reallocated, and store to the vector.
    19141917            storage = arrayStorage();
     
    19771980            break;
    19781981        }
    1979         if (i >= MIN_SPARSE_ARRAY_INDEX) {
     1982        if (indexIsSufficientlyBeyondLengthForSparseMap(i, 0) || i >= MIN_SPARSE_ARRAY_INDEX) {
    19801983            putByIndexBeyondVectorLengthWithArrayStorage(
    19811984                exec, i, value, shouldThrow, createArrayStorage(vm, 0, 0));
     
    20572060                !attributes
    20582061                && (isDenseEnoughForVector(i, storage->m_numValuesInVector))
    2059                 && increaseVectorLength(vm, i + 1))) {
     2062                && increaseVectorLength(vm, i + 1)
     2063                && !indexIsSufficientlyBeyondLengthForSparseMap(i, storage->vectorLength()))) {
    20602064            // success! - reread m_storage since it has likely been reallocated, and store to the vector.
    20612065            storage = arrayStorage();
Note: See TracChangeset for help on using the changeset viewer.