On 2013/09/23 16:53:29, Jakob wrote:
I don't see what problem this CL is solving. AFAICS,
kInitialMaxFastElementArray
is consistently used as an exclusive upper bound; this change introduces
inconsistency wrt. the use in ArrayConstructorCommon in runtime.cc.
What this patch essentially achieves is to bump the limit from 100,000 to
100,001. I don't see a reason to do that.
If I'm not mistaken, the recently added ASSERT in heap.cc is a little too
strict, we could relax it to use
FixedArray::SizeFor(JSObject::kInitialMaxFastElementArray - 1).
https://codereview.chromium.org/23441080/diff/1/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/23441080/diff/1/src/code-stubs-hydrogen.cc#newcode664
src/code-stubs-hydrogen.cc:664: HInstruction* checked_arg =
Add<HBoundsCheck>(argument, max_alloc_length + 1);
No way!
Thanks for comments.
Why I submitted this change is misc-basic-array micro in
http://www.arewefastyet.com/. I paste it below. It creates a large array
with
100000 size. V8 suffers from the size limitation (Actually at first glance,
V8
should work well in this micro, because in my memory, V8's max fast element
number could be 100000 because of the name of "kInitialMaxFastElementArray"
).
function fill_array(len) {
var a = Array(len);
for (var i = 0; i < 500; i++) {
for (var j = 0; j < len; j++)
a[j] = 0;
}
return a;
}
function use_array(a) {
for (var i = 0; i < 500; i++) {
var x = 0;
for (var j = 0; j < a.length; j++)
x += a[j];
}
}
var a = fill_array(100000);
use_array(a);
I overlooked the use in ArrayConstructorCommon in runtime.cc. (I think
useing
"len" instead of "value" there makes it more meaningful.)
https://codereview.chromium.org/23441080/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
