--verify-predictable mode added for ensuring that GC behaves deterministically.

--verify-predictable mode added for ensuring that GC behaves deterministically.
In order to be able to use it one should pass verifypredictable=on to the make tool or specify v8_enable_verify_predictable=1 in GYP_DEFINES.

R=hpayer@chromium.org

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

[Igor Sheludko, 2014-06-07 09:07:04]

PTAL

[Hannes Payer (out of office), 2014-06-09 15:23:51]

https://codereview.chromium.org/325553002/diff/1/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/325553002/diff/1/src/d8.cc#newcode292
src/d8.cc:292: // performance.now() returns a time stamp as double, measured in
milliseconds.
Please adjust the comment, i.e. say what it returns in predictable mode.

https://codereview.chromium.org/325553002/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/325553002/diff/1/src/flag-definitions.h#newco...
src/flag-definitions.h:877: DEFINE_bool(allocations_digest, false,
Do we need both, verify_predictable and allocations_digest? It seems like that
unifying both modes in verify_predictable is good enough?

https://codereview.chromium.org/325553002/diff/1/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/325553002/diff/1/src/heap.h#newcode979
src/heap.h:979: // Number of allocations done so far.
"runtime allocations"

https://codereview.chromium.org/325553002/diff/1/src/heap.h#newcode1563
src/heap.h:1563: uint32_t allocations_count_;  // how many allocations happened
Style guide: Each comment should start in a new line, upper case, ending with a
".".  Since you are here you can also change ms_count_ and gc_count_.

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h#newcode274
src/spaces-inl.h:274: heap()->OnAllocationEvent(object, size_in_bytes);
See my comment about  Heap::AllocateRaw.

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h#newcode299
src/spaces-inl.h:299: heap()->OnAllocationEvent(obj, size_in_bytes);
See my comment about  Heap::AllocateRaw.

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc#newcode2901
src/spaces.cc:2901: heap()->OnAllocationEvent(object, object_size);
The event call should be in Heap::AllocateRaw. There is actually already a call
to profiler->AllocationEvent. This is catching the same event for a different
purpose. I guess we can refactor there.

[Igor Sheludko, 2014-06-10 08:22:54]

https://codereview.chromium.org/325553002/diff/1/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/325553002/diff/1/src/d8.cc#newcode292
src/d8.cc:292: // performance.now() returns a time stamp as double, measured in
milliseconds.
On 2014/06/09 15:23:51, Hannes Payer wrote:
> Please adjust the comment, i.e. say what it returns in predictable mode.

Done.

https://codereview.chromium.org/325553002/diff/1/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/325553002/diff/1/src/flag-definitions.h#newco...
src/flag-definitions.h:877: DEFINE_bool(allocations_digest, false,
On 2014/06/09 15:23:51, Hannes Payer wrote:
> Do we need both, verify_predictable and allocations_digest? It seems like that
> unifying both modes in verify_predictable is good enough?

Done.

https://codereview.chromium.org/325553002/diff/1/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/325553002/diff/1/src/heap.h#newcode979
src/heap.h:979: // Number of allocations done so far.
On 2014/06/09 15:23:51, Hannes Payer wrote:
> "runtime allocations"

Done.

https://codereview.chromium.org/325553002/diff/1/src/heap.h#newcode1563
src/heap.h:1563: uint32_t allocations_count_;  // how many allocations happened
On 2014/06/09 15:23:51, Hannes Payer wrote:
> Style guide: Each comment should start in a new line, upper case, ending with
a
> ".".  Since you are here you can also change ms_count_ and gc_count_.

Done.

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h#newcode274
src/spaces-inl.h:274: heap()->OnAllocationEvent(object, size_in_bytes);
On 2014/06/09 15:23:51, Hannes Payer wrote:
> See my comment about  Heap::AllocateRaw.

Replied.

https://codereview.chromium.org/325553002/diff/1/src/spaces-inl.h#newcode299
src/spaces-inl.h:299: heap()->OnAllocationEvent(obj, size_in_bytes);
On 2014/06/09 15:23:51, Hannes Payer wrote:
> See my comment about  Heap::AllocateRaw.

Replied.

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc#newcode2901
src/spaces.cc:2901: heap()->OnAllocationEvent(object, object_size);
On 2014/06/09 15:23:51, Hannes Payer wrote:
> The event call should be in Heap::AllocateRaw. There is actually already a
call
> to profiler->AllocationEvent. This is catching the same event for a different
> purpose. I guess we can refactor there.

I trigger "allocation event" each time any allocation happens (either object or
target place for evacuation), while profiler has to distinguish between new
object creation and object relocation. I think that for this verification
purpose we can keep things simple. WDYT?

[Hannes Payer (out of office), 2014-06-10 16:58:59]

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/325553002/diff/1/src/spaces.cc#newcode2901
src/spaces.cc:2901: heap()->OnAllocationEvent(object, object_size);
On 2014/06/10 08:22:54, Igor Sheludko wrote:
> On 2014/06/09 15:23:51, Hannes Payer wrote:
> > The event call should be in Heap::AllocateRaw. There is actually already a
> call
> > to profiler->AllocationEvent. This is catching the same event for a
different
> > purpose. I guess we can refactor there.
> 
> I trigger "allocation event" each time any allocation happens (either object
or
> target place for evacuation), while profiler has to distinguish between new
> object creation and object relocation. I think that for this verification
> purpose we can keep things simple. WDYT?

1) Please move your monitoring code into Heap:AllocateRaw. That is our runtime
allocation choke point.
2) I would assume that it your monitoring call will be at the exact same
location as profiler->AllocationEvent. Hence, re-using this infrastructure would
keep stuff simple.
WDYT?

[Igor Sheludko, 2014-06-13 15:46:03]

I added bottlenecks for object allocation events and object movement events and
most likely fixed a bug (see comments below).

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

https://codereview.chromium.org/325553002/diff/60001/src/mark-compact.cc#newc...
src/mark-compact.cc:2875: heap()->OnMoveEvent(dst, src, size);
Before this change we were not firing 
  PROFILE(isolate, SharedFunctionInfoMoveEvent(source->address(),
          target->address()));
for such migrations unlike the ScavengingVisitor::MigrateObject(), which seems
wrong. Now we will fire.

https://codereview.chromium.org/325553002/diff/60001/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/325553002/diff/60001/src/spaces-inl.h#newcode256
src/spaces-inl.h:256: 
This change is no longer needed, however I think that new version is better.

[Hannes Payer (out of office), 2014-06-17 12:54:49]

https://codereview.chromium.org/325553002/diff/60001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/325553002/diff/60001/src/heap.cc#newcode3343
src/heap.cc:3343: OnAllocationEvent(result, object_size);
As discussed, offline: let's refactor large object allocation.

[Igor Sheludko, 2014-06-17 14:08:10]

https://codereview.chromium.org/325553002/diff/60001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/325553002/diff/60001/src/heap.cc#newcode3343
src/heap.cc:3343: OnAllocationEvent(result, object_size);
On 2014/06/17 12:54:49, Hannes Payer wrote:
> As discussed, offline: let's refactor large object allocation.

Done.

[Hannes Payer (out of office), 2014-06-18 09:45:10]

LGTM

https://codereview.chromium.org/325553002/diff/80001/src/heap-inl.h
File src/heap-inl.h (right):

https://codereview.chromium.org/325553002/diff/80001/src/heap-inl.h#newcode306
src/heap-inl.h:306: (static_cast<uint32_t>(allocation_space) << kPageSizeBits);
Why do you need the or (static_cast<uint32_t>(allocation_space) <<
kPageSizeBits);

https://codereview.chromium.org/325553002/diff/80001/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/325553002/diff/80001/src/spaces-inl.h#newcode258
src/spaces-inl.h:258: !heap()->linear_allocation() ||
I do not think the assert is correct.

I think linear_allocation will always be true.

[Igor Sheludko, 2014-06-18 12:38:49]

Done

https://codereview.chromium.org/325553002/diff/80001/src/heap-inl.h
File src/heap-inl.h (right):

https://codereview.chromium.org/325553002/diff/80001/src/heap-inl.h#newcode306
src/heap-inl.h:306: (static_cast<uint32_t>(allocation_space) << kPageSizeBits);
On 2014/06/18 09:45:10, Hannes Payer wrote:
> Why do you need the or (static_cast<uint32_t>(allocation_space) <<
> kPageSizeBits);

I did that in order to involve both object offset and allocation space kind in
hash computation.

https://codereview.chromium.org/325553002/diff/80001/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/325553002/diff/80001/src/spaces-inl.h#newcode258
src/spaces-inl.h:258: !heap()->linear_allocation() ||
On 2014/06/18 09:45:10, Hannes Payer wrote:
> I do not think the assert is correct.
> 
> I think linear_allocation will always be true.

It looks outdated so I removed both the ASSERT and the Heap::linear_allocation()
method.

[Igor Sheludko, 2014-06-20 07:36:08]

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