Move left and right trimming of FixedArray into Heap.

Move left and right trimming of FixedArray into Heap.

R=hpayer@chromium.org
BUG=v8:3231
LOG=n

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

[Hannes Payer (out of office), 2014-03-31 12:00:43]

Logically, I think LeftTrimFixedArray and RightTrimFixedArray should be part of
FixedArrayBase. But they shouldn't access GC internals from there. You could
make TrimObjectStart and TrimObjectEnd helper methods which are in heap and they
take care of free space filler creation, mark bits, live bytes count, and maybe
also heap profiler (depending how general they should be - the more general the
better). These helpers can be called from other places in object.cc as well,
i.e. all places that call CreateFillerObjectAt, which would result in a larger
dependency cleanup. WDYT?

[danno-g, 2014-04-01 13:45:10]

Hmm. I am not sure I agree, Hannes. Why should they be in FixedArray? The
trimming contains lots of implementation details of the GC, so that doesn't
belong in object.* or FixedArray, IMO, it belongs in the GC, which subscribes to
the minimum of information from objects.h/FixedArrayBase to get it's job done.

Litmus test: If you replaced our current GC with a different one, which files
would you intuitively think you have to change? object.* or heap.cc? Seems like
heap.cc to me.

[Hannes Payer (out of office), 2014-04-01 15:29:55]

On 2014/04/01 13:45:10, danno-g wrote:
> Hmm. I am not sure I agree, Hannes. Why should they be in FixedArray? The
> trimming contains lots of implementation details of the GC, so that doesn't
> belong in object.* or FixedArray, IMO, it belongs in the GC, which subscribes
to
> the minimum of information from objects.h/FixedArrayBase to get it's job done.
> 
> Litmus test: If you replaced our current GC with a different one, which files
> would you intuitively think you have to change? object.* or heap.cc? Seems
like
> heap.cc to me.

I am not saying they should be part of object.*. I am saying that the actual
FixedArray
specific code should be in object.* and the garbage collector related code
should be
in an abstract Trim* method in heap.*.

[Michael Starzinger, 2014-08-05 10:32:10]

As discussed offline, I revived and rebased this CL. PTAL.

[Hannes Payer (out of office), 2014-08-05 10:45:07]

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3291: Object** zap =
reinterpret_cast<Object**>(object->address());
Please add a a method that is taking care of the zaping which is called from
both left and right trimming.

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3374: // Force instantiation of template instances class.
Please move the force instantiation before the actual instantiation.

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.h#newcode708
src/heap/heap.h:708: // Maintain consistency of live bytes during incremental
marking.
Move the enum before the comment and describe in a brief comment what the enum
is used for.

[Michael Starzinger, 2014-08-05 11:01:06]

Addressed comments.

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3291: Object** zap =
reinterpret_cast<Object**>(object->address());
On 2014/08/05 10:45:07, Hannes Payer wrote:
> Please add a a method that is taking care of the zaping which is called from
> both left and right trimming.

Done. I called it ZapFixedArrayForTrimming.

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3374: // Force instantiation of template instances class.
On 2014/08/05 10:45:07, Hannes Payer wrote:
> Please move the force instantiation before the actual instantiation.

Done.

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/200443004/diff/40001/src/heap/heap.h#newcode708
src/heap/heap.h:708: // Maintain consistency of live bytes during incremental
marking.
On 2014/08/05 10:45:07, Hannes Payer wrote:
> Move the enum before the comment and describe in a brief comment what the enum
> is used for.

Done.

[Hannes Payer (out of office), 2014-08-05 11:08:15]

LGTM

https://codereview.chromium.org/200443004/diff/60001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/200443004/diff/60001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3300: ZapFixedArrayForTrimming(object->address(),
elements_to_trim);
As discussed offline, zapping should not be needed anymore. We should remove it
in a separate cl.

https://codereview.chromium.org/200443004/diff/60001/src/heap/heap.cc#newcode...
src/heap/heap.cc:3357: ZapFixedArrayForTrimming(new_end, elements_to_trim);
Same same.

[Michael Starzinger, 2014-08-05 11:16:20]

Committed patchset #3 manually as 22858 (presubmit successful).