Optimize calls to object literal properties that are initialized with a funct...

Optimize calls to object literal properties that are initialized with a function literal.

This allows fast calls and inlining of functions like:

var o = {f: function() { return "foo"; }}
o.f();


Object literals that contain function literals are initially created a dictionary mode
object and only transformed to fast properties once all properties are computed and
added. This allows us to create constant function properties for functions declared
inside the object literal. Function literals inside object literals are marked for
pretenuring so that they work as contant function properties.

Object literals without functions should just function as before.

Committed: http://code.google.com/p/v8/source/detail?r=7283

[fschneider, 2011-01-27 11:29:14]

This is a proposal to speed up calls to functions defined as object literal
properties.

[Mads Ager (chromium), 2011-01-27 14:12:17]

Let's keep iteration order in this case.

Can we make the construction of the fast-case map insert constant function
descriptors for the properties with the hole value. We should know the index of
those statically and then we can just overwrite the hole with the actual
functions in the map in the generated code after creating the map?

[fschneider, 2011-02-01 13:31:20]

Uploaded a new version. It preserves the order of properties and should not
affect object literals that don't contain functions.

[Mads Ager (chromium), 2011-02-01 13:50:59]

A couple of comments. Also, you should modify x64 and arm as well.

http://codereview.chromium.org/6240012/diff/6001/src/handles.cc
File src/handles.cc (left):

http://codereview.chromium.org/6240012/diff/6001/src/handles.cc#oldcode871
src/handles.cc:871: OptimizedObjectForAddingMultipleProperties::
Is there a change to the header file missing here. If you remove the
implementation you should remove the declaration as well.

Getting rid of the longest class name in the system. Nice. ;)

http://codereview.chromium.org/6240012/diff/6001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:1405: if (expr->fast_elements() &&
expr->has_function()) {
I'm not sure I understand why fast elements is part of this condition? The
function part is about the properties which should have nothing to do with the
elements? Shouldn't this just check expr->has_function and not care about the
elements?

http://codereview.chromium.org/6240012/diff/6001/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:3362: }
Shouldn't this contain the call to go back to fast elements as well?

http://codereview.chromium.org/6240012/diff/6001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/runtime.cc#newcode307
src/runtime.cc:307: // slow properties mode for now and we don't go in the map
cache.
Please expand the comment to say that we cannot share the maps in any case
because the constant functions will rarely (never?) be the same.

[fschneider, 2011-02-02 10:18:38]

Addressed comments.

I added a new HIR/LIR instruction to enable the same optimization in optimized
code and I'm now pretenuring function literals inside object literals.

http://codereview.chromium.org/6240012/diff/6001/src/handles.cc
File src/handles.cc (left):

http://codereview.chromium.org/6240012/diff/6001/src/handles.cc#oldcode871
src/handles.cc:871: OptimizedObjectForAddingMultipleProperties::
On 2011/02/01 13:51:00, Mads Ager wrote:
> Is there a change to the header file missing here. If you remove the
> implementation you should remove the declaration as well.
> 
> Getting rid of the longest class name in the system. Nice. ;)

Done.

http://codereview.chromium.org/6240012/diff/6001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:1405: if (expr->fast_elements() &&
expr->has_function()) {
On 2011/02/01 13:51:00, Mads Ager wrote:
> I'm not sure I understand why fast elements is part of this condition? The
> function part is about the properties which should have nothing to do with the
> elements? Shouldn't this just check expr->has_function and not care about the
> elements?

Absolutely right. Done.

http://codereview.chromium.org/6240012/diff/6001/src/ia32/lithium-codegen-ia3...
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:3362: }
On 2011/02/01 13:51:00, Mads Ager wrote:
> Shouldn't this contain the call to go back to fast elements as well?

Yes, in fact I need to insert some new instruction to do the call after all
properties are evaluated.

http://codereview.chromium.org/6240012/diff/6001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6240012/diff/6001/src/runtime.cc#newcode307
src/runtime.cc:307: // slow properties mode for now and we don't go in the map
cache.
On 2011/02/01 13:51:00, Mads Ager wrote:
> Please expand the comment to say that we cannot share the maps in any case
> because the constant functions will rarely (never?) be the same.

Done.

[fschneider, 2011-02-02 12:49:39]

added arm and x64 ports.

[Mads Ager (chromium), 2011-02-02 13:07:52]

Almost there. One question below.

http://codereview.chromium.org/6240012/diff/6004/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6240012/diff/6004/src/hydrogen-instructions.h#...
src/hydrogen-instructions.h:3085: class HToFastProperties: public
HUnaryOperation {
Is it OK that this is not marked as changing maps when that is exactly what it
is doing? I'm concerned that maybe code that relies on the new map can be
hoisted to before this instruction where the map is not the same?

[fschneider, 2011-02-02 14:23:32]

I also fixed the use of the pretenure-flags on function literals in the lithium
codegen.

Please have a look.

http://codereview.chromium.org/6240012/diff/6004/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6240012/diff/6004/src/hydrogen-instructions.h#...
src/hydrogen-instructions.h:3085: class HToFastProperties: public
HUnaryOperation {
On 2011/02/02 13:07:52, Mads Ager wrote:
> Is it OK that this is not marked as changing maps when that is exactly what it
> is doing? I'm concerned that maybe code that relies on the new map can be
> hoisted to before this instruction where the map is not the same?

As discussed I'm making the return value of the ToFastProperties-call the result
of the whole object literal expression and assert that the HToFastProperties is
only used with an freshly created object literal as input.

[Mads Ager (chromium), 2011-02-02 14:29:05]

LGTM

Good catch on the pretenuring!

Kevin, if you have cycles it might be good if you double check the safety of not
marking the 'to fast properties' instruction as changing maps.