Lars,
couple of notes.
That currently introduces performance hit for Dromaeo's arrays test (<2%).
I
hope I know how to fix it:
1) introduce special generators for CallIC for some functions (putting them
into
shared info)---that should allow to implement ArrayPush and ArrayPop in
plain
assembler for the fastest case;
2) put functions on both hidden and normal prototypes (saves exactly the map
check we're now paying for).
Regarding benchmarks: do I get you right that you want me to add another
benchmark not into v8 itself, but in perf monitoring infrastructure?
http://codereview.chromium.org/660298/diff/4/5
File src/bootstrapper.cc (right):
http://codereview.chromium.org/660298/diff/4/5#newcode1465
src/bootstrapper.cc:1465: Handle<JSFunction>
Genesis::MakeFunction(Handle<String> name,
On 2010/03/02 13:07:56, bak wrote:
You should change the name of this method to be more specific.
Done. Is it fine or you want something more specific?
http://codereview.chromium.org/660298/diff/4/5#newcode1502
src/bootstrapper.cc:1502: Handle<JSFunction>
old(JSFunction::cast(prototype->GetProperty(*key)));
On 2010/03/02 13:07:56, bak wrote:
You might want an explicit check in debug mode to ensure the original
function
is there.
I didn't find concise way to do that. My plan was to fetch property
from Top::global_context()->builtins(), but the problem is methods and
builtins names are different: push vs. ArrayPush etc. Of course, it's
easy to make one from another, but check becomes too heavyweight imho.
Still, if you think we'd better have one, I'll implement it.
Another approach might be to expose runtime function which accepts
builtin name and returns C++ builtin for this name. In this approach
array.js prototype initialization code would look like:
"join": ArrayJoin,
...
"push": %CppBuiltin("ArrayPush"),
which is easier to comprehend for me.
http://codereview.chromium.org/660298/diff/4/6
File src/builtins.cc (right):
http://codereview.chromium.org/660298/diff/4/6#newcode348
src/builtins.cc:348: if (!args.receiver()->IsJSArray()) {
On 2010/03/02 13:07:56, bak wrote:
It would be easier to read if you could combine the check:
Object* rec = args.receiver();
if (!rec->IsJSArray() || JSArray::cast(rec)->HasFastElements()) {
// Add nice comment here.
... bail out...
}
Done.
http://codereview.chromium.org/660298
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
