Make cells pointing to JSObjects weak in optimized code.

Make cells pointing to JSObjects weak in optimized code.

This is done similar to weak embedded objects in optimized code (r17102). The
reference from optimized code to a cell is treated weakly in marking visitors
if the cell points to a JSObject. After marking we iterate over all cells
embedded in optimized code. If a cell is not marked but its value is marked,
then we revive the cell by marking it. Otherwise, the cell value is dead, so
we mark the code for deoptimization.

BUG=v8:2073
TEST=cctest/test-heap/CellsInOptimizedCodeAreWeak
LOG=Y
R=hpayer@chromium.org, mstarzinger@chromium.org

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

[ulan, 2013-12-19 13:27:09]

Please take a look.

[Michael Starzinger, 2014-01-10 13:45:19]

LGTM (with once comment to address and one comment for posterity).

https://codereview.chromium.org/117483002/diff/20001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/117483002/diff/20001/src/heap.cc#newcode3311
src/heap.cc:3311: 
nit: Only one empty new-line here.

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

https://codereview.chromium.org/117483002/diff/20001/src/objects-visiting-inl...
src/objects-visiting-inl.h:263:
heap->mark_compact_collector()->RecordRelocSlot(rinfo, object);
Side-note: A potential cleanup/optimization for later would be to only records
slots to surviving objects here (i.e. move the RecordRelocSlot into the
conditional) and instead record slots while we clear non-live pointers.

But that is tricky business and we should give the code some breathing time
before we tackle it.

https://codereview.chromium.org/117483002/diff/20001/src/objects-visiting-inl...
src/objects-visiting-inl.h:275: if
(!Code::IsWeakEmbeddedObject(rinfo->host()->kind(), cell)) {
As discussed offline: We do not record slots in code that point to cells. That
is _only_ correct because we don't compact cell space. We should put a comment
here explaining this, so that we remember once fragmentation of cell space
becomes an issue.

[ulan, 2014-01-13 11:35:06]

Thanks, Michael. I added comments and rebased.

https://codereview.chromium.org/117483002/diff/20001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/117483002/diff/20001/src/heap.cc#newcode3311
src/heap.cc:3311: 
On 2014/01/10 13:45:19, Michael Starzinger wrote:
> nit: Only one empty new-line here.

Done.

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

https://codereview.chromium.org/117483002/diff/20001/src/objects-visiting-inl...
src/objects-visiting-inl.h:263:
heap->mark_compact_collector()->RecordRelocSlot(rinfo, object);
On 2014/01/10 13:45:19, Michael Starzinger wrote:
> Side-note: A potential cleanup/optimization for later would be to only records
> slots to surviving objects here (i.e. move the RecordRelocSlot into the
> conditional) and instead record slots while we clear non-live pointers.
> 
> But that is tricky business and we should give the code some breathing time
> before we tackle it.

Added TODO with this message.

https://codereview.chromium.org/117483002/diff/20001/src/objects-visiting-inl...
src/objects-visiting-inl.h:275: if
(!Code::IsWeakEmbeddedObject(rinfo->host()->kind(), cell)) {
On 2014/01/10 13:45:19, Michael Starzinger wrote:
> As discussed offline: We do not record slots in code that point to cells. That
> is _only_ correct because we don't compact cell space. We should put a comment
> here explaining this, so that we remember once fragmentation of cell space
> becomes an issue.

Done.

[ulan, 2014-01-13 12:57:53]

Turns out cells can be marked as embedded objects in reloc info. Uploaded patch
that handles that.

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

LGTM, test nits.

https://codereview.chromium.org/117483002/diff/150001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/117483002/diff/150001/test/cctest/test-heap.c...
test/cctest/test-heap.cc:3705: heap->CollectAllGarbage(false);
Is calling 3 times the GC not good enough? Moreover, I would call
CollectAllGarbage with Heap::kAbortIncrementalMarkingMask to make it more robust
on the stress GC builders.

https://codereview.chromium.org/117483002/diff/150001/test/cctest/test-heap.c...
test/cctest/test-heap.cc:3746: heap->CollectAllGarbage(false);
Same as above.

[ulan, 2014-01-14 17:13:59]

Thanks!

https://codereview.chromium.org/117483002/diff/150001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/117483002/diff/150001/test/cctest/test-heap.c...
test/cctest/test-heap.cc:3705: heap->CollectAllGarbage(false);
I am passing Heap::kAbortIncrementalMarkingMask now.

I left the number of GCs at four, because three GCs are not collecting the
function.

[ulan, 2014-01-15 11:42:31]

Committed patchset #5 manually as r18616 (presubmit successful).