Prevent modification of cached normalized maps....

Prevent modification of cached normalized maps.

Finally sovles the problem that r5342 attempted to solve.
When adding a stub to a map's code cache we need to make
sure that this map is not used by object that do not need
this stub.

Existing solution had 2 flaws:
1. It checked that the map is cached by asking the current context.
If the object escaped into another context then NormalizedMapCache::Contains
returns false negative.

2. If a map gets evicted from the cache we should not try to modify it
even though Contains returns false.

This patch implements much less fragile solution of the same problem:
A map now has a flag (is_shared) that is set once the map is added
to a cache, stays set even after the cache eviction, and is cleared
if the object goes back to fast mode.

Added a regression test.


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

[Vyacheslav Egorov (Chromium), 2010-09-23 17:17:51]

LGTM!

http://codereview.chromium.org/3472006/diff/9001/8008
File src/objects.cc (right):

http://codereview.chromium.org/3472006/diff/9001/8008#newcode2177
src/objects.cc:2177: // cleared if the object goes back to fast properties.
This comment is slightly confusing. The flag itself is cleared [not set] on the
copy of the map and the map pointer of the object is replaced. 

The comment should probably just state that fast maps cannot be marked as
shared.

http://codereview.chromium.org/3472006/diff/9001/8008#newcode8781
src/objects.cc:8781: obj->map()->set_is_shared(false);
This seems to be a weird place for this. 

Should not we clean this bit in the very place where map is copied?
[CopyDropDescriptors]

http://codereview.chromium.org/3472006/diff/9001/8009
File src/objects.h (right):

http://codereview.chromium.org/3472006/diff/9001/8009#newcode3204
src/objects.h:3204: bool shared);
Consider using enum instead of boolean to improve callsites readability.

[Vladislav Kaznacheev, 2010-09-23 19:45:09]

Thanks for the comments. Please review for naming clarity.

http://codereview.chromium.org/3472006/diff/9001/8008
File src/objects.cc (right):

http://codereview.chromium.org/3472006/diff/9001/8008#newcode2177
src/objects.cc:2177: // cleared if the object goes back to fast properties.
On 2010/09/23 17:17:52, Vyacheslav Egorov wrote:
> This comment is slightly confusing. The flag itself is cleared [not set] on
the
> copy of the map and the map pointer of the object is replaced. 
> 
> The comment should probably just state that fast maps cannot be marked as
> shared.

Done.

http://codereview.chromium.org/3472006/diff/9001/8008#newcode8781
src/objects.cc:8781: obj->map()->set_is_shared(false);
On 2010/09/23 17:17:52, Vyacheslav Egorov wrote:
> This seems to be a weird place for this. 
> 
> Should not we clean this bit in the very place where map is copied?
> [CopyDropDescriptors]

Done.

http://codereview.chromium.org/3472006/diff/9001/8009
File src/objects.h (right):

http://codereview.chromium.org/3472006/diff/9001/8009#newcode3204
src/objects.h:3204: bool shared);
On 2010/09/23 17:17:52, Vyacheslav Egorov wrote:
> Consider using enum instead of boolean to improve callsites readability.

Done.

[Vyacheslav Egorov (Chromium), 2010-09-23 21:06:06]

Still LGTM with a linter error in test-api.cc fixed [some stray whitespaces]