Changed the global object representation

Changed the global object representation.The global object is now always in dictionary (slow) mode with each of its properties stored in a cell object. A cell object has one field containing the actual value for the property. Inline caches for access to global properties which uses direct to the cell are now created for load, store and call to properties of the global object. When properties of the global object are deleted the cell for that property is kept with an indcation of that the property is deleted.Added counters to track the use of the global property inline caches.Added additional information on IC's in the disassembler.

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

[Kasper Lund, 2009-06-29 12:52:55]

LGTM.

http://codereview.chromium.org/151019/diff/1/18
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/151019/diff/1/18#newcode76
Line 76: __ ldr(t0, FieldMemOperand(t1, JSObject::kMapOffset));
We're really loading the same map of bunch of times here...

http://codereview.chromium.org/151019/diff/1/18#newcode84
Line 84: __ cmp(r3, Operand(Factory::hash_table_map()));
Is there anyway we could make this check fail instead of having a separate check
for global objects?

http://codereview.chromium.org/151019/diff/1/17
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/151019/diff/1/17#newcode709
Line 709: 
Remove one of the newlines.

http://codereview.chromium.org/151019/diff/1/3
File src/heap.cc (right):

http://codereview.chromium.org/151019/diff/1/3#newcode1076
Line 1076: global_property_value_map_ = Map::cast(obj);
Maybe this should be renamed to global_property_cell_map_?

http://codereview.chromium.org/151019/diff/1/3#newcode2084
Line 2084: // Make sure result is NOT a JS global object if valid.
Very weird comment. Wrong.

http://codereview.chromium.org/151019/diff/1/5
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/151019/diff/1/5#newcode940
Line 940: __ mov(edx, eax);  // Bug that we cannot use eax below?
Maybe add a TODO to investigate this issue?

http://codereview.chromium.org/151019/diff/1/8
File src/ic.cc (right):

http://codereview.chromium.org/151019/diff/1/8#newcode425
Line 425: case NORMAL: {
Would it make sense to have a GLOBAL lookup type here instead of re-using the
NORMAL case?

[Mads Ager (chromium), 2009-06-29 13:59:06]

There is a security check issue with this change.  Through the API, we allow
enabling access checks on the global object at any time.  As we discussed
offline, we should perform a map check instead of identity check.

http://codereview.chromium.org/151019/diff/1/11
File src/objects.h (right):

http://codereview.chromium.org/151019/diff/1/11#newcode3082
Line 3082: // Retrieve the property cel used to store a property.
cel -> cell.

[Søren Thygesen Gjesse, 2009-06-30 09:53:44]

Changed the identity check for the global object into a map check to handle the
security issue involved when the global object is detached and no longer has a
global proxy. Added a test for this as well.

http://codereview.chromium.org/151019/diff/1/18
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/151019/diff/1/18#newcode76
Line 76: __ ldr(t0, FieldMemOperand(t1, JSObject::kMapOffset));
On 2009/06/29 12:52:55, Kasper Lund wrote:
> We're really loading the same map of bunch of times here...

Fixed by keeping the map in r3.

Also fixed the check for the has_named_interceptor bit, which was wrong.

http://codereview.chromium.org/151019/diff/1/18#newcode84
Line 84: __ cmp(r3, Operand(Factory::hash_table_map()));
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Is there anyway we could make this check fail instead of having a separate
check
> for global objects?

Will look into that in a separate CL.

http://codereview.chromium.org/151019/diff/1/17
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/151019/diff/1/17#newcode709
Line 709: 
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Remove one of the newlines.

Done.

http://codereview.chromium.org/151019/diff/1/3
File src/heap.cc (right):

http://codereview.chromium.org/151019/diff/1/3#newcode1076
Line 1076: global_property_value_map_ = Map::cast(obj);
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Maybe this should be renamed to global_property_cell_map_?

Done.

http://codereview.chromium.org/151019/diff/1/3#newcode2084
Line 2084: // Make sure result is NOT a JS global object if valid.
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Very weird comment. Wrong.

Done.

http://codereview.chromium.org/151019/diff/1/5
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/151019/diff/1/5#newcode940
Line 940: __ mov(edx, eax);  // Bug that we cannot use eax below?
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Maybe add a TODO to investigate this issue?

This was not a bug after all. Other uses of RecordWrite also passed to value to
store in a scratch register. Comment updated.

http://codereview.chromium.org/151019/diff/1/8
File src/ic.cc (right):

http://codereview.chromium.org/151019/diff/1/8#newcode425
Line 425: case NORMAL: {
On 2009/06/29 12:52:55, Kasper Lund wrote:
> Would it make sense to have a GLOBAL lookup type here instead of re-using the
> NORMAL case?

It does, but when I looked into PropertyType it currently has eight values
stored in the bit field PropertyDetails::TypeField. And I did not want to
allocate another bit yust for that.

http://codereview.chromium.org/151019/diff/1/11
File src/objects.h (right):

http://codereview.chromium.org/151019/diff/1/11#newcode3082
Line 3082: // Retrieve the property cel used to store a property.
On 2009/06/29 13:59:06, Mads Ager wrote:
> cel -> cell.

Done.

[Mads Ager (chromium), 2009-06-30 10:07:32]

LGTM, thanks!

http://codereview.chromium.org/151019/diff/1014/1036
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/151019/diff/1014/1036#newcode6152
Line 6152: // inline cache is used.
remove 'is used'

http://codereview.chromium.org/151019/diff/1014/1036#newcode6153
Line 6153: CHECK(!f1->Call(global, 0, NULL)->IsUndefined());
Maybe check that it is actually '1' in these cases?