Implement crankshaft support for nested object literals.

Implement crankshaft support for nested object literals.

This generates optimized code for deep-copying of nested object literal
boilerplates which are statically known. Most of the boilerplates have
already been generated at crankshaft time, so this optimization should
kick in for virtually every object literal. Only nested object literal
graphs up to a certain depth and containing up to a certain total number
of properties are considered for this optimization. This will prevent
explosion of code size due to large object literals (e.g. eval on JSON).
Improves splay performance because object literals are created often.

R=fschneider@chromium.org

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

[Michael Starzinger, 2011-11-22 12:29:03]

The current patch set will not compile on x64 and ARM, because lithium parts are
not yet ported. But it would be cool to get and initial review before I start
porting stuff.

[fschneider, 2011-11-22 17:26:16]

This is already pretty good.

I have some suggestions for improvements which should make it even faster.

http://codereview.chromium.org/8640001/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/8640001/diff/2001/src/hydrogen.cc#newcode3363
src/hydrogen.cc:3363: Handle<JSFunction> closure = info()->closure();
You probably should write:

Handle<JSFunction> closure = function_state()->compilation_info()->closure();

FunctionState gives you the correct state if you're inside an inlined function
body. (Right now we don't inline functions with materialized literals yet, so it
does not matter. But if we do, you have to change it to get the correct closure)

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

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4194: for (int i = 0; i < inobject_properties;
i++) {
Can you instead of cloning the original, just store constants into the target
area? All values should be known at compile-time, right? No need for source
register then.

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4201: __ lea(ecx, Operand(result, *offset));
Replace the 3 lines above with:

__ LoadHeapObject(ecx, HeapObject::cast(value));

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4206: __ mov(ecx, FieldOperand(source,
in_object_offset));
Here you could say:

if (value->IsHeapObject()) {
  __ LoadHeapObject(ecx, HeapObject::cast(value));
} else {
  __ mov(ecx, Immediate(value));  
}

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4233: __ mov(ebx, FieldOperand(ebx,
literal_offset));
You could just load the boilerplace object directly. The LoadHeapObject macro
handles the case of a new-space object by indirecting via a global cell:

__ LoadHeapObject(ebx, instr->hydrogen()->boilerplate());

[Michael Starzinger, 2011-11-23 10:55:32]

Added new patch set. Will start porting to x64 and ARM.

http://codereview.chromium.org/8640001/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/8640001/diff/2001/src/hydrogen.cc#newcode3363
src/hydrogen.cc:3363: Handle<JSFunction> closure = info()->closure();
On 2011/11/22 17:26:16, fschneider wrote:
> You probably should write:
> 
> Handle<JSFunction> closure = function_state()->compilation_info()->closure();
> 
> FunctionState gives you the correct state if you're inside an inlined function
> body. (Right now we don't inline functions with materialized literals yet, so
it
> does not matter. But if we do, you have to change it to get the correct
closure)

Done.

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

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4194: for (int i = 0; i < inobject_properties;
i++) {
On 2011/11/22 17:26:16, fschneider wrote:
> Can you instead of cloning the original, just store constants into the target
> area? All values should be known at compile-time, right? No need for source
> register then.

Done.

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4201: __ lea(ecx, Operand(result, *offset));
On 2011/11/22 17:26:16, fschneider wrote:
> Replace the 3 lines above with:
> 
> __ LoadHeapObject(ecx, HeapObject::cast(value));

Done. As discussed offline, ecx needs to be loaded with a pointer into the space
just allocated in DoObjectLiteralFast(), which is not known statically. But we
can still get rid of the pushes and pops of the source register.

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4206: __ mov(ecx, FieldOperand(source,
in_object_offset));
On 2011/11/22 17:26:16, fschneider wrote:
> Here you could say:
> 
> if (value->IsHeapObject()) {
>   __ LoadHeapObject(ecx, HeapObject::cast(value));
> } else {
>   __ mov(ecx, Immediate(value));  
> }

Done.

http://codereview.chromium.org/8640001/diff/2001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4233: __ mov(ebx, FieldOperand(ebx,
literal_offset));
On 2011/11/22 17:26:16, fschneider wrote:
> You could just load the boilerplace object directly. The LoadHeapObject macro
> handles the case of a new-space object by indirecting via a global cell:
> 
> __ LoadHeapObject(ebx, instr->hydrogen()->boilerplate());

Done.

[fschneider, 2011-11-23 14:27:20]

LGTM with a few comments.

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

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4186: object->elements()->map() ==
HEAP->fixed_cow_array_map());
Use isolate()->heap() instead.

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4208: __ mov(ecx, Immediate(value));
Instead of using ecx, you can use immediate operand directly:

__ mov(FieldOperand(result, current_offset + in_object_offset),
Immediate(value));

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4210: __ mov(FieldOperand(result,
current_offset + in_object_offset), ecx);
Move this into the heap-object case of the if-statement.

[Michael Starzinger, 2011-11-24 09:51:54]

Ported to x64 and ARM.

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

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4186: object->elements()->map() ==
HEAP->fixed_cow_array_map());
On 2011/11/23 14:27:20, fschneider wrote:
> Use isolate()->heap() instead.

Done.

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4208: __ mov(ecx, Immediate(value));
On 2011/11/23 14:27:20, fschneider wrote:
> Instead of using ecx, you can use immediate operand directly:
> 
> __ mov(FieldOperand(result, current_offset + in_object_offset),
> Immediate(value));

Done.

http://codereview.chromium.org/8640001/diff/6001/src/ia32/lithium-codegen-ia3...
src/ia32/lithium-codegen-ia32.cc:4210: __ mov(FieldOperand(result,
current_offset + in_object_offset), ecx);
On 2011/11/23 14:27:20, fschneider wrote:
> Move this into the heap-object case of the if-statement.

Done.

[fschneider, 2011-11-24 11:25:53]

LGTM as well.