Preserve optimized code map during GCs weakly.

Preserve optimized code map during GCs weakly.

This change preserves the contents of optimized code maps during GCs but
treats the references in this cache weakly. It uses infrastructure from
code flushing to maintain a list of all caches.

R=hpayer@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=14695

[Michael Starzinger, 2013-05-14 13:32:59]

Architecture ports still missing, but ready for a first round.

[Michael Starzinger, 2013-05-14 18:25:05]

Just a heads up now that the architecture ports are done ...

https://codereview.chromium.org/14794007/diff/3001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/14794007/diff/3001/src/objects.cc#newcode8998
src/objects.cc:8998: new_code_map = FACTORY->NewFixedArray(new_length);
There is a bug in this part of the code as the call to Factory::NewFixedArray
might trigger a GC and hence cause trimming of old_code_map. This means that the
below call to Heap::CopyTo might copy beyond the actual array. This is covered
by our tests only on ARM in release mode due to different GC timings. I am going
to work on a fix tomorrow.

[Michael Starzinger, 2013-05-15 09:44:40]

Fix uploaded ...

https://codereview.chromium.org/14794007/diff/3001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/14794007/diff/3001/src/objects.cc#newcode8998
src/objects.cc:8998: new_code_map = FACTORY->NewFixedArray(new_length);
On 2013/05/14 18:25:05, Michael Starzinger wrote:
> There is a bug in this part of the code as the call to Factory::NewFixedArray
> might trigger a GC and hence cause trimming of old_code_map. This means that
the
> below call to Heap::CopyTo might copy beyond the actual array. This is covered
> by our tests only on ARM in release mode due to different GC timings. I am
going
> to work on a fix tomorrow.

Fixed in patch set 3.

[Hannes Payer (out of office), 2013-05-15 13:11:25]

LGTM! just a few nits...

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1072: Code* code = Code::cast(code_map->get(i + 1));
Can we use a constant like kCodeOffset = 1 here? The same comment would apply
for the the code_map->get calls below. I think it improves readability.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1081: code_map->set(new_length++, code_map->get(i));
kContextOffset = 0

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1090: code_map->set(new_length++, code_map->get(i + 1));
kCodeOffset = 1

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1099: code_map->set(new_length++, code_map->get(i + 2));
kLiteralsOffset = 2

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1202: }
A follow-up CL could refactor the list handling code.

https://codereview.chromium.org/14794007/diff/10001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/objects.cc#newcode8998
src/objects.cc:8998: MaybeObject* maybe = heap->AllocateFixedArray(kEntriesStart
+ kEntryLength);
can we have another constant for that kEntriesStart + kEntryLength, like
kFixedArraySize (kSecondEntryIndex)

https://codereview.chromium.org/14794007/diff/10001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/x64/code-stubs-x64.cc...
src/x64/code-stubs-x64.cc:303: __ subq(rdx,
Immediate(SharedFunctionInfo::kEntryLength));  // Skip an entry.
Do you want to delete the comment, for consistency reasons.

[Michael Starzinger, 2013-05-15 15:09:57]

Addressed comments. Will land after some more benchmarking.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1072: Code* code = Code::cast(code_map->get(i + 1));
On 2013/05/15 13:11:25, Hannes Payer wrote:
> Can we use a constant like kCodeOffset = 1 here? The same comment would apply
> for the the code_map->get calls below. I think it improves readability.

Done.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1081: code_map->set(new_length++, code_map->get(i));
On 2013/05/15 13:11:25, Hannes Payer wrote:
> kContextOffset = 0

Done.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1090: code_map->set(new_length++, code_map->get(i + 1));
On 2013/05/15 13:11:25, Hannes Payer wrote:
> kCodeOffset = 1

Done.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1099: code_map->set(new_length++, code_map->get(i + 2));
On 2013/05/15 13:11:25, Hannes Payer wrote:
> kLiteralsOffset = 2

Done.

https://codereview.chromium.org/14794007/diff/10001/src/mark-compact.cc#newco...
src/mark-compact.cc:1202: }
On 2013/05/15 13:11:25, Hannes Payer wrote:
> A follow-up CL could refactor the list handling code.

Agreed. If have two ideas for cleanup:
1) Come up with a OptimizedCodeMap class to handle all accesses.
2) Factor out the weak slot handling in the code-flusher and other places.

https://codereview.chromium.org/14794007/diff/10001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/objects.cc#newcode8998
src/objects.cc:8998: MaybeObject* maybe = heap->AllocateFixedArray(kEntriesStart
+ kEntryLength);
On 2013/05/15 13:11:25, Hannes Payer wrote:
> can we have another constant for that kEntriesStart + kEntryLength, like
> kFixedArraySize (kSecondEntryIndex)

Done. Named the constant kInitialLength.

https://codereview.chromium.org/14794007/diff/10001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/14794007/diff/10001/src/x64/code-stubs-x64.cc...
src/x64/code-stubs-x64.cc:303: __ subq(rdx,
Immediate(SharedFunctionInfo::kEntryLength));  // Skip an entry.
On 2013/05/15 13:11:25, Hannes Payer wrote:
> Do you want to delete the comment, for consistency reasons.

Done.

[Michael Starzinger, 2013-05-15 16:09:35]

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