Optimize testing for an index's existence in packed Arrays

Optimize testing for an index's existence in packed Arrays

This patch introduces a new inline runtime function,
%_HasFastPackedElements(), and uses it both in the implementation
of the 'in' operator and in the array builtins to speed
up testing for the existence of an index in an array.

In testing with the microbenchmark on the attached bug,
for example, the runtime goes from 326ms to 66ms.

A reviewer might ask whether the HAS_INDEX macro is worthwhile,
and I tried the same example without it, which pushed the
microbenchmark up to 157ms. So it seems it's worth it to
avoid the function call to IN() if we know we're dealing
with arrays and numbers.

BUG=v8:3701
LOG=n

[adamk, 2014-11-27 00:36:55]

adamk@chromium.org changed reviewers:
+ verwaest@chromium.org

[adamk, 2014-11-27 00:36:56]

Hi Toon, not sure who to send this to (it's my first time adding such a
function). Feel free to redirect as appropriate (and no hurry, I won't get back
to look at this till next week).

[Dmitry Lomov (no reviews), 2014-11-27 07:17:54]

dslomov@chromium.org changed reviewers:
+ dslomov@chromium.org

[Dmitry Lomov (no reviews), 2014-11-27 07:17:54]

https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h#n...
src/runtime/runtime.h:708: F(HasFastPackedElements, 1, 1)
Is it critical for HasFastPackedElements to be INLINE_FUNCTION instead of
INLINE_OPTIMIZED_FUNCTION (see below, line 718)? 
If you use runtime function in full codegen, you will save yourself from having
to write all those tedious Emit functions in all the ports

[adamk, 2014-12-02 19:36:21]

https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h#n...
src/runtime/runtime.h:708: F(HasFastPackedElements, 1, 1)
On 2014/11/27 07:17:54, Dmitry Lomov (chromium) wrote:
> Is it critical for HasFastPackedElements to be INLINE_FUNCTION instead of
> INLINE_OPTIMIZED_FUNCTION (see below, line 718)? 
> If you use runtime function in full codegen, you will save yourself from
having
> to write all those tedious Emit functions in all the ports

Not sure what counts as "critical". Sure, the implementations are a bit tedious,
but they're straightforward enough, so I don't see much point in penalizing full
codegen if I can make them fast.

[Dmitry Lomov (no reviews), 2014-12-02 19:59:40]

On 2014/12/02 19:36:21, adamk wrote:
> https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h
> File src/runtime/runtime.h (right):
> 
>
https://codereview.chromium.org/754863002/diff/120001/src/runtime/runtime.h#n...
> src/runtime/runtime.h:708: F(HasFastPackedElements, 1, 1)
> On 2014/11/27 07:17:54, Dmitry Lomov (chromium) wrote:
> > Is it critical for HasFastPackedElements to be INLINE_FUNCTION instead of
> > INLINE_OPTIMIZED_FUNCTION (see below, line 718)? 
> > If you use runtime function in full codegen, you will save yourself from
> having
> > to write all those tedious Emit functions in all the ports
> 
> Not sure what counts as "critical". Sure, the implementations are a bit
tedious,
> but they're straightforward enough, so I don't see much point in penalizing
full
> codegen if I can make them fast.

I meant critical for performance (you are after speeding up the benchmark, so
'critical' means 'if you replace this with runtime function in full codegen,
does the benchmark suffer?').
Re  straightforwardness: 
a) We have quite a few ports even beyond the 4 you have covered, if we can avoid
adding this to every port, it is a win
b) Ports beget complexity, complexity begets bugs, possibly security bugs

[adamk, 2014-12-02 21:10:14]

Okay, I've reverted the full codegen bits for now (as this particular benchmark
doesn't show any speed hit with only the hydrogen version available).

[adamk, 2014-12-02 22:36:51]

The CQ bit was checked by adamk@chromium.org

[adamk, 2014-12-02 22:36:54]

The CQ bit was unchecked by adamk@chromium.org

[Dmitry Lomov (no reviews), 2014-12-04 11:59:41]

lgtm

[Dmitry Lomov (no reviews), 2014-12-04 12:00:07]

On 2014/12/04 11:59:41, Dmitry Lomov (chromium) wrote:
> lgtm

(sorry for delay in review, I was sick..)

[adamk, 2014-12-04 18:20:09]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2014-12-04 18:20:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/754863002/180001

[commit-bot: I haz the power, 2014-12-04 18:46:41]

Committed patchset #10 (id:180001)