Don't overwrite transition array map while iterating over the transition tree.

Don't overwrite transition array map while iterating over the transition tree.

Instread of using the transition array map to store the iteration index, we are using the constructor field of the map. Moreover, I removed the unchecked transition array accessors.

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

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

[Hannes Payer (out of office), 2014-04-03 08:38:03]

I think I can also take advantage of the back pointer - no need to overwrite the
map of the map. WDYT?

[Hannes Payer (out of office), 2014-04-03 10:45:25]

The back pointer simplification does not work, since maps in the prototype
transition array do not point back to the parent.

[Toon Verwaest, 2014-04-03 11:06:23]

Overall looks good to me, but can you please ASSERT that all constructors are
actually equal?

[Michael Starzinger, 2014-04-03 13:07:36]

LGTM (👀👍). I like it. Just a couple of nits. Only one real suggestion about the
ChildIteratorStart function.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7251
src/objects.cc:7251: explicit IntrusiveMapTransitionIterator(
nit: Doesn't need to be "explicit" any longer.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7260
src/objects.cc:7260: *IteratorField() = Smi::FromInt(-1);
Can we ASSERT(*IteratorField() == constructor_) here before storing the smi?

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7272
src/objects.cc:7272: int index = (value * -1) - 1;
nit: Would "-value - 1" be easier to read?

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7295
src/objects.cc:7295: // of map while it is running.  Positive values in the
constructor field
nit: s/of map/of the map/

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7300
src/objects.cc:7300: explicit IntrusivePrototypeTransitionIterator(
nit: Doesn't need to be "explicit" any longer.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7306
src/objects.cc:7306: *IteratorField() = Smi::FromInt(0);
Can we ASSERT(*IteratorField() == constructor_) here before storing the smi?

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7392
src/objects.cc:7392: void ChildIteratorStart(Object* constructor) {
It might make it more uniform to to get rid of the explicit ChildIteratorStart()
and do implicit StartOfNotStarted() within ChildIteratorNext() for both of the
sub-iterators. WDYT?

[Hannes Payer (out of office), 2014-04-03 20:01:34]

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7251
src/objects.cc:7251: explicit IntrusiveMapTransitionIterator(
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> nit: Doesn't need to be "explicit" any longer.

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7260
src/objects.cc:7260: *IteratorField() = Smi::FromInt(-1);
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> Can we ASSERT(*IteratorField() == constructor_) here before storing the smi?

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7272
src/objects.cc:7272: int index = (value * -1) - 1;
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> nit: Would "-value - 1" be easier to read?

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7295
src/objects.cc:7295: // of map while it is running.  Positive values in the
constructor field
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> nit: s/of map/of the map/

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7300
src/objects.cc:7300: explicit IntrusivePrototypeTransitionIterator(
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> nit: Doesn't need to be "explicit" any longer.

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7306
src/objects.cc:7306: *IteratorField() = Smi::FromInt(0);
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> Can we ASSERT(*IteratorField() == constructor_) here before storing the smi?

Done.

https://codereview.chromium.org/223533002/diff/20001/src/objects.cc#newcode7392
src/objects.cc:7392: void ChildIteratorStart(Object* constructor) {
On 2014/04/03 13:07:37, Michael Starzinger wrote:
> It might make it more uniform to to get rid of the explicit
ChildIteratorStart()
> and do implicit StartOfNotStarted() within ChildIteratorNext() for both of the
> sub-iterators. WDYT?

Done. That is an awesome comment, thanks! Note that it is not completely
symmetric, since I cannot ASSERT(!(*IteratorField())->IsSmi() || IsIterating());
in the IntrusivePrototypeTransitionIterator.

[Toon Verwaest, 2014-04-04 09:10:53]

lgtm

[Hannes Payer (out of office), 2014-04-04 09:25:46]

Committed patchset #4 manually as r20503 (presubmit successful).