Simplify and compact transitions storage

Simplify and compact transitions storage

Simple transitions are now stored in a map's "transitions" field (as a WeakCell wrapping the target map); full TransitionArrays are used when that's not sufficient.
To encapsulate these storage format implementation details, functions for manipulating and querying transitions have been refactored to be static functions on the TransitionArray class, and take maps as inputs.

Committed: https://crrev.com/45fbef7f2252fce10634931cb103ccc1fc95ae6a
Cr-Commit-Position: refs/heads/master@{#27029}

[Jakob Kummerow, 2015-03-04 12:05:35]

jkummerow@chromium.org changed reviewers:
+ verwaest@chromium.org

[Jakob Kummerow, 2015-03-04 12:05:35]

Toon, PTAL.
Ulan: FYI.

Sorry that the patch is so large; I don't see a good way to split it up.
At least there are two points of comfort:
(1) The biggest part of the modified line count is caused by moving code around
(out of objects.h/cc into transitions.h/cc; some reshuffling within
transitions.h).
(2) There's no intended change in observable semantics (i.e. which transitions
are stored, cleared, and retrieved at which time).

There is some potential for further trimming down the public API of
TransitionArray, if the interface to the garbage collector is redesigned. I'm
not sure that's worth it; either way I'm leaving it for future CLs.

[Toon Verwaest, 2015-03-05 13:18:54]

Overall looks like a big improvement. Some nits, plus as discussed, I'd prefer
it if the api would always take a Map* rather than raw_transitions(). I think
only transitions.* should look at raw_transitions().

https://codereview.chromium.org/980573002/diff/1/src/heap/objects-visiting-inl.h
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/980573002/diff/1/src/heap/objects-visiting-in...
src/heap/objects-visiting-inl.h:590: heap, HeapObject::RawField(map,
Map::kTransitionsOffset));
This keeps all simple transitions alive?

https://codereview.chromium.org/980573002/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2319
src/objects.cc:2319: Map* m = TransitionArray::GetTarget(transitions, i);
write out names fully: map

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2562
src/objects.cc:2562: if (m == NULL) break;
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2649
src/objects.cc:2649: Map* m = TransitionArray::SearchTransition(
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2964
src/objects.cc:2964: Map* m = TransitionArray::SearchTransition(new_map,
old_details.kind(),
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode5728
src/objects.cc:5728: if (m != NULL) {
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode5795
src/objects.cc:5795: if (m != NULL) {
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode7355
src/objects.cc:7355: Map* m = TransitionArray::SearchTransition(*map, kData,
*name, attributes);
same here

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode7447
src/objects.cc:7447: TransitionArray::SearchTransition(*map, kAccessor, *name,
attributes);
same here

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc
File src/transitions.cc (right):

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc#newcode24
src/transitions.cc:24: map->set_raw_transitions(*Map::WeakCellForMap(target));
Handle<WeakCell> cell = Map::WeakCellForMap(target);
map->set_raw_transitions(*cell);

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc#newcode44
src/transitions.cc:44: map->set_raw_transitions(*Map::WeakCellForMap(target));
Same as above

[Jakob Kummerow, 2015-03-05 15:26:37]

Addressed your comments. As discussed offline, I'd prefer to do the
raw_transitions -> map API change in a separate CL.

https://codereview.chromium.org/980573002/diff/1/src/heap/objects-visiting-inl.h
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/980573002/diff/1/src/heap/objects-visiting-in...
src/heap/objects-visiting-inl.h:590: heap, HeapObject::RawField(map,
Map::kTransitionsOffset));
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> This keeps all simple transitions alive?

No, the WeakCell takes care of that. (We must keep the WeakCell itself alive
here.)

https://codereview.chromium.org/980573002/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2319
src/objects.cc:2319: Map* m = TransitionArray::GetTarget(transitions, i);
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> write out names fully: map

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2562
src/objects.cc:2562: if (m == NULL) break;
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2649
src/objects.cc:2649: Map* m = TransitionArray::SearchTransition(
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode2964
src/objects.cc:2964: Map* m = TransitionArray::SearchTransition(new_map,
old_details.kind(),
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode5728
src/objects.cc:5728: if (m != NULL) {
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode5795
src/objects.cc:5795: if (m != NULL) {
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode7355
src/objects.cc:7355: Map* m = TransitionArray::SearchTransition(*map, kData,
*name, attributes);
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/objects.cc#newcode7447
src/objects.cc:7447: TransitionArray::SearchTransition(*map, kAccessor, *name,
attributes);
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> same here

Done.

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc
File src/transitions.cc (right):

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc#newcode24
src/transitions.cc:24: map->set_raw_transitions(*Map::WeakCellForMap(target));
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> Handle<WeakCell> cell = Map::WeakCellForMap(target);
> map->set_raw_transitions(*cell);

Done.

https://codereview.chromium.org/980573002/diff/1/src/transitions.cc#newcode44
src/transitions.cc:44: map->set_raw_transitions(*Map::WeakCellForMap(target));
On 2015/03/05 13:18:54, Toon Verwaest wrote:
> Same as above

Done.

[Toon Verwaest, 2015-03-05 16:20:45]

lgtm

[ulan, 2015-03-05 16:24:13]

ulan@chromium.org changed reviewers:
+ ulan@chromium.org

[ulan, 2015-03-05 16:24:13]

gc parts lgtm

[Jakob Kummerow, 2015-03-05 17:53:35]

Patchset #3 (id:40001) has been deleted

[Toon Verwaest, 2015-03-05 17:56:23]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2015-03-05 17:56:23]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/980573002/#ps60001 (title:
"cleaned up ClearMapTransitions() as discussed")

[commit-bot: I haz the power, 2015-03-05 17:57:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/980573002/60001

[commit-bot: I haz the power, 2015-03-05 18:09:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-05 18:09:58]

Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[Jakob Kummerow, 2015-03-05 19:45:09]

For the record: patch set 4 includes a rebase (sorry!); the real diff to patch
set 3 is one less ';' in objects-inl.h

[Jakob Kummerow, 2015-03-05 19:45:16]

The CQ bit was checked by jkummerow@chromium.org

[Jakob Kummerow, 2015-03-05 19:45:17]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/980573002/#ps80001 (title:
"fix -Werror=pedantic ( rebase")

[commit-bot: I haz the power, 2015-03-05 19:45:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/980573002/80001

[commit-bot: I haz the power, 2015-03-05 20:09:33]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-03-05 20:09:56]

Patchset 4 (id:??) landed as
https://crrev.com/45fbef7f2252fce10634931cb103ccc1fc95ae6a
Cr-Commit-Position: refs/heads/master@{#27029}

[Jakob Kummerow, 2015-03-05 20:39:38]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/982143002/ by jkummerow@chromium.org.

The reason for reverting is: x64 test failures.