Allow race-full access of map instance size when sweeping concurrently.

Allow race-full access of map instance size when sweeping concurrently.

BUG=
R=jarin@chromium.org, mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=20604

[Hannes Payer (out of office), 2014-04-07 12:24:46]

There a two problems with that cl:
1) The instance size accessors are part of the objects API. We should remove
these accessors as soon as we have the proper abstractions on the GC side.
2) The atomic byte accessors are ugly, but we cannot make them part of the
atomics API since we took that code from chromium. If we have to access another
atomic byte somewhere in the code, we should probably factor the accessors out.

[Hannes Payer (out of office), 2014-04-07 12:31:12]

https://codereview.chromium.org/227133007/diff/20001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/227133007/diff/20001/src/objects-inl.h#newcod...
src/objects-inl.h:4075: size_field >>= 8;
Do you prefer masking ifdef style?

[Hannes Payer (out of office), 2014-04-07 19:33:42]

unused_property_fields and inobject_properties also have to be annotated. I will
update the cl before you can review it.

[Michael Starzinger, 2014-04-08 11:34:21]

LGTM. Another complexity-induced brain explosion.

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcod...
src/objects-inl.h:4315: int difference = (slack << 8) + slack;
I don't even ... OMG.

https://codereview.chromium.org/227133007/diff/70001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/227133007/diff/70001/src/objects.cc#newcode10227
src/objects.cc:10227: map->Shrink(slack);
nit: Can we call that Map::ShrinkInstanceSize for readability?

[Hannes Payer (out of office), 2014-04-08 13:12:54]

I don't like it, we should do something cleaner. Maybe use the TSAN byte
accessors...

[Jarin, 2014-04-08 20:11:09]

I am not too happy about the assertions. Otherwise, it is looking good.

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcod...
src/objects-inl.h:4057: ASSERT(kInstanceSizeOffset % kPointerSize == 0);
STATIC_ASSERT?

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcod...
src/objects-inl.h:4303: ASSERT(kInstanceSizeOffset % kPointerSize == 0);
STATIC_ASSERT? Tiny nit - since you are using Atomic32, you only really need
kInstanceSizeOffset % sizeof(Atomic32) == 0.

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcod...
src/objects-inl.h:4315: int difference = (slack << 8) + slack;
Replace 8 with  kInObjectPropertiesByte * kBitsPerByte, also might want
STATIC_ASSERT that kInObjectPropertiesByte < sizeof(Atomic32)

https://codereview.chromium.org/227133007/diff/70001/src/objects-inl.h#newcod...
src/objects-inl.h:4320: ((first_word & 0xff00) >> 8) < 256);
The two assertions above do not seem to test any interesting property, this
should hold for any value of first_word, no? 

Maybe we want an assertion testing that slack <= object_size and slack <=
in_object_properties. I was imagining something like this: (inserted before we
do first_word -= difference)

ASSERT(slack <= (first_word & 0xff));
ASSERT(slack <= ((first_word >> kInObjectPropertiesByte * kBitsPerByte) &
0xff));

[Hannes Payer (out of office), 2014-04-09 09:28:00]

Thx Jaro for the comments, but as discussed yesterday I decided to redo this cl
based on atomic byte accessors which makes the original cl obsolete.

Please check out the new version.

[Jarin, 2014-04-09 09:41:44]

lgtm

[Hannes Payer (out of office), 2014-04-09 09:50:32]

Committed patchset #7 manually as r20604 (presubmit successful).