Fix clearing of dead dependent codes and verify weak embedded maps on full GC.

Fix clearing of dead dependent codes and verify weak embedded maps on full GC.

BUG=172488, 172489
R=mstarzinger@chromium.org

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

[ulan, 2013-01-29 15:02:57]

Please take a look. The "weak embedded maps" flag is still set to false by
default.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/lithium.cc
File src/lithium.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/lithium.cc#newc...
src/lithium.cc:459: NoWeakEmbeddedMapsVerificationScope
disable_verification_of_embedded_maps;
I tried putting AlwaysAllocateScope here, but then some cc tests fail because
they already have AlwaysAllocateScope and we cannot nest them.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:2335: Object** slot =
codes->code_slot_at(new_number_of_codes);
This was the bug that lead to crashes.

[Michael Starzinger, 2013-01-31 14:27:50]

LGTM (with a couple of nits).

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/lithium.cc
File src/lithium.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/lithium.cc#newc...
src/lithium.cc:459: NoWeakEmbeddedMapsVerificationScope
disable_verification_of_embedded_maps;
On 2013/01/29 15:02:58, ulan wrote:
> I tried putting AlwaysAllocateScope here, but then some cc tests fail because
> they already have AlwaysAllocateScope and we cannot nest them.

Yeah, it's unfortunate that we need a specialized scope for that, but I don't
see another solution either, so I am fine with it.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:837: static void VerifyWeakEmbeddedMapsInOptimizedCode(Heap*
heap) {
Move this function up to the other verifiers at the top of the file.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:870: if (FLAG_collect_maps &&
FLAG_weak_embedded_maps_in_optimized_code &&
Move this call into MarkCompactCollector::CollectGarbage() where the other
verifiers are called.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:2335: Object** slot =
codes->code_slot_at(new_number_of_codes);
On 2013/01/29 15:02:58, ulan wrote:
> This was the bug that lead to crashes.

Ouch, nice catch, I should have seen that in my initial review.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:2344: number_of_codes = codes->number_of_codes();
This call seems to be obsolete.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects-inl.h#n...
src/objects-inl.h:80: ASSERT(object->Is##type());                 \
Unnecessary white-space change.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc#newc...
src/objects.cc:9212: void Code::VerifyEmbeddedMaps() {
This whole implementation belongs into objects-debug.cc I guess.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc#newc...
src/objects.cc:9218: Map* map = Map::cast(it.rinfo()->target_object());
Indentation is off.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.h#newco...
src/objects.h:4549: void VerifyEmbeddedMaps();
Let's call this "VerifyEmbeddedMapsDependency()".

[ulan, 2013-02-04 09:54:06]

Thanks, I uploaded new patch set.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:837: static void VerifyWeakEmbeddedMapsInOptimizedCode(Heap*
heap) {
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> Move this function up to the other verifiers at the top of the file.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:870: if (FLAG_collect_maps &&
FLAG_weak_embedded_maps_in_optimized_code &&
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> Move this call into MarkCompactCollector::CollectGarbage() where the other
> verifiers are called.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/mark-compact.cc...
src/mark-compact.cc:2344: number_of_codes = codes->number_of_codes();
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> This call seems to be obsolete.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects-inl.h#n...
src/objects-inl.h:80: ASSERT(object->Is##type());                 \
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> Unnecessary white-space change.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc#newc...
src/objects.cc:9212: void Code::VerifyEmbeddedMaps() {
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> This whole implementation belongs into objects-debug.cc I guess.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.cc#newc...
src/objects.cc:9218: Map* map = Map::cast(it.rinfo()->target_object());
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> Indentation is off.

Done.

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/12094036/diff/2001/src/objects.h#newco...
src/objects.h:4549: void VerifyEmbeddedMaps();
On 2013/01/31 14:27:50, Michael Starzinger wrote:
> Let's call this "VerifyEmbeddedMapsDependency()".

Done.