Bug 196055 – Cap length of an array with spread to MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

We are doing this because:
1. We expect the array to be densely packed.
2. SpeculativeJIT::compileAllocateNewArrayWithSize() (and the FTL equivalent) expects the array length to be less than MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH if we don't want to use an ArrayStorage shape.
3. There's no reason why an array with spread needs to be that large anyway.  MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH is plenty.

<rdar://problem/49067448>

[Mark Lam]

Created attachment 365484 [details]
proposed patch.

Let's try this on the EWS first.

[Mark Lam]

Comment on attachment 365484 [details]
proposed patch.

The JSC tests run to completion locally without any failures.  Let's get a review.

[Yusuke Suzuki]

Comment on attachment 365484 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=365484&action=review

r=me

> Source/JavaScriptCore/dfg/DFGOperations.cpp:2727
> +    }

If some program hits this, we could

1. make `length >= MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH` OSR exit with Overflow (this is already done in this patch)
2. In operationNewArrayWithSpreadSlow, we return some information, and cause OSR exit with Overflow
3. In baseline / LLInt, we just allocate ArrayStorage JSArray
4. avoids emitting NewArrayWithSpread DFG nodes if hasExitSite(Overflow) = true in DFG

but I think throwing OOM error is OK until we find some real programs hit this condition.

[Mark Lam]

Thanks for the review.  Landed in r243280: <http://trac.webkit.org/r243280>.