[webkit-reviews] review granted: [Bug 133011] JSArray::shiftCountWith* could be more efficient : [Attachment 231592] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 133011: JSArray::shiftCountWith* could be more efficient
https://bugs.webkit.org/show_bug.cgi?id=133011

Attachment 231592: Patch
https://bugs.webkit.org/attachment.cgi?id=231592&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231592&action=review


r=me

> Source/JavaScriptCore/ChangeLog:10
> +	   them to correctly handle holes, thus avoiding the slowest of slow
paths in most cases.

What is the speedup?

Do we need a js/regress test?

> Source/JavaScriptCore/runtime/JSArray.cpp:680
> +	   || inSparseIndexingMode() 
> +	   || shouldUseSlowPut(indexingType())) {

I would move these checks into canUseFastShift, or rename canUseFastShift to
something like mightInterceptIndexedAccess.

> Source/JavaScriptCore/runtime/JSArray.cpp:1700
> +bool JSArray::canUseFastShift(VM& vm) const

Alternatively, maybe this function should go next to
Structure::anyObjectInChainMayInterceptIndexedAccesses in Structure.