On 2013/08/14 07:48:47, Michael Starzinger wrote:
https://codereview.chromium.org/22545007/diff/20001/src/array.js
File src/array.js (right):
https://codereview.chromium.org/22545007/diff/20001/src/array.js#newcode1263
src/array.js:1263: if (!isProxy && %HasFastPackedElements(array) || i in
array)
{
On 2013/08/13 20:47:42, Toon Verwaest wrote:
> I don't think you can have indexed accessors while being packed. I
think we
> always go dictionary mode when accessors are installed. Holey arrays
would
have
> this problem; but they are already excluded explicitly.
>
> On 2013/08/13 15:47:16, Michael Starzinger wrote:
> > On 2013/08/13 14:10:22, Michael Starzinger wrote:
> > > It is _really_ surprising that the runtime call into
> > > Runtime_HasFastPackedElements is cheaper than the "in" keyword. This
> suggests
> > > that there is a lot of potential in actually improving "in" in
general.
This
> > > needs investigation of the generated code!
> >
> > Now that I think about it, I don't actually think that this is a valid
> > optimization either. What happens if the callback actually changes the
array
> in
> > a way so that it remains in fast-packed mode, but "i" still runs out
of
> bounds.
> > Imagine the following callback.
> >
> > var array = [1,2,3,4,5,6];
> > Object.defineProperty(array, '1', {
> > get: function () { array.length = 3; },
> > configurable: true
> > });
> > array.forEach(f);
>
OK, admittedly, my example does not trigger the problem and is
unnecessarily
complex. But you don't need accessors to reproduce this problem ...
var array = [1,2,3,4,5,6];
function f(value, index, array) {
array.length = 3;
}
array.forEach(f);
Then we could compare the length, but only if the length is a data
property...
it gets ugly fast... but it gives us 2x performance in the common case
where an
array is used.
Another thing we could do is to special case packed JSArray in HasElement.
My
experiments show that gives a 30% perf improvement for forEach but it should
benefit all in operations for arrays. Something like:
bool JSReceiver::HasElement(uint32_t index) {
if (IsJSArray()) {
if (JSObject::cast(this)->HasFastPackedElements()) {
int len = Smi::cast(JSArray::cast(this)->length())->value();
return index < static_cast<uint32_t>(len);
}
}
if (IsJSProxy()) {
return JSProxy::cast(this)->HasElementWithHandler(index);
}
return JSObject::cast(this)->GetElementAttributeWithReceiver(
this, index, true) != ABSENT;
}
https://codereview.chromium.org/22545007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
