Fix the heap profiler crash caused by memory layout changes between passes.

Fix the heap profiler crash caused by memory layout changes between passes.

The heap profiler randomly crashed because of memory corruption caused
by unexpected heap objects layout changes occured between count and fill
passes. The changes lead the number of retainers counted on the first pass
did not match its number on the fill pass leading to the out of bounds
array access.

Besides that the mark bit scheme has been changed to a plain vector one in
dominators building algorithm. It is up to 4x faster because of smaller
memory access footprint.

BUG=
TEST=

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

[alexeif, 2012-03-05 15:17:16]

Hi Misha,

Could you please take a look.

Thank you!

[mnaganov (inactive), 2012-03-05 16:21:58]

Thanks for digging into this issue! Please sort out GenerateSnapshot sequence
and you are done.

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (left):

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1674: } else if (object->IsJSGlobalObject()) {
Will this fall through down to the generic case that creates "object,
HeapEntry::kHidden"? Should you create a stub entry instead that you will
properly tag in the final pass?

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.c...
src/profile-generator.cc:3165: &&
v8_heap_explorer_.IterateAndSetObjectNames(&filler);
As we have discussed offline, please split GenerateSnapshot into 3 parts:

1. TagGlobalObjects and everything else that changes the heap
== Garbage collection ==
2. Local scope block protected by AssertNoAllocations, running serialization
code assuming that no heap modifications are made.
3. Code that can again can cause heap allocations / reallocations.

[alexeif, 2012-03-05 17:35:31]

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (left):

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.c...
src/profile-generator.cc:1674: } else if (object->IsJSGlobalObject()) {
On 2012/03/05 16:21:58, Mikhail Naganov (Chromium) wrote:
> Will this fall through down to the generic case that creates "object,
> HeapEntry::kHidden"? Should you create a stub entry instead that you will
> properly tag in the final pass?
It should fall into JSObject at 1703 which is the superclass for JSGLobalObject.
The code for both cases seems to be the same now, so I killed this one.

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/9594020/diff/1/src/profile-generator.c...
src/profile-generator.cc:3165: &&
v8_heap_explorer_.IterateAndSetObjectNames(&filler);
On 2012/03/05 16:21:58, Mikhail Naganov (Chromium) wrote:
> As we have discussed offline, please split GenerateSnapshot into 3 parts:
> 
> 1. TagGlobalObjects and everything else that changes the heap
> == Garbage collection ==
> 2. Local scope block protected by AssertNoAllocations, running serialization
> code assuming that no heap modifications are made.
> 3. Code that can again can cause heap allocations / reallocations.
I checked the AssertNoAllocation class and it doesn't seem to make any
difference here.
The code that failed does not make any new allocations, it instead reassigns a
reference from one object to another. But it was enough to fail, because the
number of retainers has changed between count and fill passes.
So I think it's ok to leave it as is, and keep the AssertNoAllocation cover the
region it currently covers. WDYT?

[mnaganov (inactive), 2012-03-05 18:02:56]

Thanks for explanations!

LGTM, I'm submitting.