Change code pointer in function objects to a pointer to the first...

Change code pointer in function objects to a pointer to the first
instruction.

By changing the pointer to the code object to a pointer to the first
instruction we can call directly this instruction directly instead of
looking up the address through the code object.


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

[Rico, 2010-08-18 13:18:23]

Erik: Could you take a look at the snapshot part?

Slave: Could you take a look at the GC part?

Kasper: Could you take a look at everything :-)

[Kasper Lund, 2010-08-18 14:25:10]

LGTM. Shouldn't it be possible to simplify the builtin function/code tables and
only have the functions again?

http://codereview.chromium.org/3156028/diff/6002/12003
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/3156028/diff/6002/12003#newcode1501
src/arm/macro-assembler-arm.cc:1501: 
Extra newline here.

http://codereview.chromium.org/3156028/diff/6002/12011
File src/objects-inl.h (right):

http://codereview.chromium.org/3156028/diff/6002/12011#newcode2749
src/objects-inl.h:2749: return reinterpret_cast<Code*>(
Hmm. This looks like it could be the same as code() - which is prettier, but
maybe you should change code() to use a Code::cast(...) instead of the
reinterpret_cast<Code*>().

http://codereview.chromium.org/3156028/diff/6002/12013
File src/objects.h (right):

http://codereview.chromium.org/3156028/diff/6002/12013#newcode3714
src/objects.h:3714: // through pointers to the first instruction int the code
object.
int => in

http://codereview.chromium.org/3156028/diff/6002/12018
File src/x64/macro-assembler-x64.cc (right):

http://codereview.chromium.org/3156028/diff/6002/12018#newcode592
src/x64/macro-assembler-x64.cc:592: 
Extra newline.

[Rico, 2010-08-18 15:43:46]

http://codereview.chromium.org/3156028/diff/6002/12003
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/3156028/diff/6002/12003#newcode1501
src/arm/macro-assembler-arm.cc:1501: 
On 2010/08/18 14:25:10, Kasper Lund wrote:
> Extra newline here.

Done.

http://codereview.chromium.org/3156028/diff/6002/12011
File src/objects-inl.h (right):

http://codereview.chromium.org/3156028/diff/6002/12011#newcode2749
src/objects-inl.h:2749: return reinterpret_cast<Code*>(
On 2010/08/18 14:25:10, Kasper Lund wrote:
> Hmm. This looks like it could be the same as code() - which is prettier, but
> maybe you should change code() to use a Code::cast(...) instead of the
> reinterpret_cast<Code*>().
Yes, changed unchecked_code to the code version and used Code::cast in code().

http://codereview.chromium.org/3156028/diff/6002/12013
File src/objects.h (right):

http://codereview.chromium.org/3156028/diff/6002/12013#newcode3714
src/objects.h:3714: // through pointers to the first instruction int the code
object.
On 2010/08/18 14:25:10, Kasper Lund wrote:
> int => in

Done.

http://codereview.chromium.org/3156028/diff/6002/12018
File src/x64/macro-assembler-x64.cc (right):

http://codereview.chromium.org/3156028/diff/6002/12018#newcode592
src/x64/macro-assembler-x64.cc:592: 
On 2010/08/18 14:25:10, Kasper Lund wrote:
> Extra newline.

Done.

[Vyacheslav Egorov (Chromium), 2010-08-18 15:46:59]

GC part LGTM

[Rico, 2010-08-19 06:12:44]

Kasper: Regarding the functions/code table for builtins, you are absolutely
right. I will do this in another change.

[Erik Corry, 2010-08-19 11:14:59]

LGTM

http://codereview.chromium.org/3156028/diff/23001/24014
File src/serialize.cc (right):

http://codereview.chromium.org/3156028/diff/23001/24014#newcode866
src/serialize.cc:866: ONE_PER_SPACE(kNewObject, kPlain, kFirstInstruction)
It's a bit wasteful in terms of V8's exe size to have a case per space here when
only two spaces can be involved (code and large code).  Might be worth trying
ALL_SPACES instead or if that impacts startup speed making a new
ONE_PER_CODE_SPACE.

[Vyacheslav Egorov (Chromium), 2010-08-19 11:38:25]

some additional nitpicking

http://codereview.chromium.org/3156028/diff/23001/24011
File src/objects-inl.h (right):

http://codereview.chromium.org/3156028/diff/23001/24011#newcode2406
src/objects-inl.h:2406: Object* Code::GetObjectFromEntryAddress(Address entry) {
Name 'entry' is slightly confusing because it is not entry address but address
of location where entry address is stored.

http://codereview.chromium.org/3156028/diff/23001/24011#newcode2741
src/objects-inl.h:2741: Address entry =
You can implement code() as

return Code::cast(unchecked_code());

do avoid code duplication.

http://codereview.chromium.org/3156028/diff/23001/24011#newcode2751
src/objects-inl.h:2751: HeapObject::FromAddress(entry - Code::kHeaderSize));
Can you use GetObjectFromEntryAddress here?

[Rico, 2010-08-20 06:59:57]

Thanks for the extra comment Slava - fixed.

http://codereview.chromium.org/3156028/diff/23001/24011
File src/objects-inl.h (right):

http://codereview.chromium.org/3156028/diff/23001/24011#newcode2741
src/objects-inl.h:2741: Address entry =
On 2010/08/19 11:38:26, Vyacheslav Egorov wrote:
> You can implement code() as
> 
> return Code::cast(unchecked_code());
> 
> do avoid code duplication.

Done.

http://codereview.chromium.org/3156028/diff/23001/24011#newcode2751
src/objects-inl.h:2751: HeapObject::FromAddress(entry - Code::kHeaderSize));
On 2010/08/19 11:38:26, Vyacheslav Egorov wrote:
> Can you use GetObjectFromEntryAddress here? 

Done.

[Rico, 2010-08-20 07:01:48]

Thanks for comments Erik, I created a new ONE_PER_CODE_SPACE

http://codereview.chromium.org/3156028/diff/23001/24014
File src/serialize.cc (right):

http://codereview.chromium.org/3156028/diff/23001/24014#newcode866
src/serialize.cc:866: ONE_PER_SPACE(kNewObject, kPlain, kFirstInstruction)
On 2010/08/19 11:14:59, Erik Corry wrote:
> It's a bit wasteful in terms of V8's exe size to have a case per space here
when
> only two spaces can be involved (code and large code).  Might be worth trying
> ALL_SPACES instead or if that impacts startup speed making a new
> ONE_PER_CODE_SPACE.

Done.