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#newcode459
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#newcode837
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#newcode870
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#newcode2335
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#newcode2344
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#newcode80
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#newcode9212
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#newcode9218
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#newcode4549
src/objects.h:4549: void VerifyEmbeddedMaps();
Let's call this "VerifyEmbeddedMapsDependency()".
https://chromiumcodereview.appspot.com/12094036/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
