v8: make GCTracer persistent.

v8: make GCTracer persistent.

GCTracer will become the central place for logging statistics for
scheduled garbage collection. This patch makes GCTracer a member of
Heap, so that it is always available for logging.

R=hpayer@chromium.org,jochen@chromium.org
BUG=

[ernstm, 2014-07-14 11:49:02]

PTAL

[jochen (gone - plz use gerrit), 2014-07-14 11:55:33]

lgtm

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

https://codereview.chromium.org/390823003/diff/1/src/heap.h#newcode578
src/heap.h:578: kNumberOfScopes
it's a bit odd to mix MACRO_STYLE and kStyle in the same enum?

https://codereview.chromium.org/390823003/diff/1/src/heap.h#newcode596
src/heap.h:596: };
DISALLOW_COPY_AND_ASSIGN()

https://codereview.chromium.org/390823003/diff/1/src/heap.h#newcode706
src/heap.h:706: };
disallopw copy/assign

[Michael Starzinger, 2014-07-14 12:04:31]

We can perform multiple scavenges while the incremental marker (and hence a
mark-and-compact GC) is still in progress. Has this been taken into account? How
will this affect the numbers tracked by the persistent GCTracer?

[Michael Starzinger, 2014-07-14 12:18:29]

Also another point, as discussed offline: This seems to be intended to use the
numbers gathered by the GCTracer for actual heuristics used in GC timing
decisions. This is something we avoided so far (unless I missed a change). All
counters used for heuristics are either in Heap or MarkCompactCollector
directly. It might be worth considering to keep this separation.

[ernstm, 2014-07-14 13:45:16]

On 2014/07/14 12:04:31, Michael Starzinger wrote:
> We can perform multiple scavenges while the incremental marker (and hence a
> mark-and-compact GC) is still in progress. Has this been taken into account?
How
> will this affect the numbers tracked by the persistent GCTracer?

This patch shouldn't change the numbers, unless I missed something. The new
start
and stop methods implement the functionality previously found in the
constructor and destructor of GCTracer. The incremental marking step counters
are now
tracked directly in GCTracer, instead of copying them from IncrementalMarking in
the constructor.

[ernstm, 2014-07-14 13:51:47]

On 2014/07/14 12:18:29, Michael Starzinger wrote:
> Also another point, as discussed offline: This seems to be intended to use the
> numbers gathered by the GCTracer for actual heuristics used in GC timing
> decisions. This is something we avoided so far (unless I missed a change). All
> counters used for heuristics are either in Heap or MarkCompactCollector
> directly. It might be worth considering to keep this separation.

The motivation for this approach is to avoid code duplication. The stats
required for
debugging and for timing decisions will overlap to a certain extent.

[Michael Starzinger, 2014-07-14 13:54:03]

On 2014/07/14 13:45:16, ernstm wrote:
> On 2014/07/14 12:04:31, Michael Starzinger wrote:
> > We can perform multiple scavenges while the incremental marker (and hence a
> > mark-and-compact GC) is still in progress. Has this been taken into account?
> How
> > will this affect the numbers tracked by the persistent GCTracer?
> 
> This patch shouldn't change the numbers, unless I missed something. The new
> start
> and stop methods implement the functionality previously found in the
> constructor and destructor of GCTracer. The incremental marking step counters
> are now
> tracked directly in GCTracer, instead of copying them from IncrementalMarking
in
> the constructor.

Yes, the mechanics of which numbers are gathered where didn't change. But the
change in line 1107 of heap.cc no longer installs a different GCTracer object
for the scavenge operation. Instead the GCTracer object for a mark-and-compact
GC is shared with the one for a scavenge operation. So the statistics gathered
while scavenging are gathered on top of any running mark-and-compact operation.

[ernstm, 2014-07-14 14:34:40]

On 2014/07/14 13:54:03, Michael Starzinger wrote:
> On 2014/07/14 13:45:16, ernstm wrote:
> > On 2014/07/14 12:04:31, Michael Starzinger wrote:
> > > We can perform multiple scavenges while the incremental marker (and hence
a
> > > mark-and-compact GC) is still in progress. Has this been taken into
account?
> > How
> > > will this affect the numbers tracked by the persistent GCTracer?
> > 
> > This patch shouldn't change the numbers, unless I missed something. The new
> > start
> > and stop methods implement the functionality previously found in the
> > constructor and destructor of GCTracer. The incremental marking step
counters
> > are now
> > tracked directly in GCTracer, instead of copying them from
IncrementalMarking
> in
> > the constructor.
> 
> Yes, the mechanics of which numbers are gathered where didn't change. But the
> change in line 1107 of heap.cc no longer installs a different GCTracer object
> for the scavenge operation. Instead the GCTracer object for a mark-and-compact
> GC is shared with the one for a scavenge operation. So the statistics gathered
> while scavenging are gathered on top of any running mark-and-compact
operation.

Got it. I assumed that there was no overlap between what is traced in Scavenge()
and MarkCompact(). The overlap makes it difficult to use a single GCTracer.

[Hannes Payer (out of office), 2014-07-14 14:40:10]

On 2014/07/14 13:54:03, Michael Starzinger wrote:
> On 2014/07/14 13:45:16, ernstm wrote:
> > On 2014/07/14 12:04:31, Michael Starzinger wrote:
> > > We can perform multiple scavenges while the incremental marker (and hence
a
> > > mark-and-compact GC) is still in progress. Has this been taken into
account?
> > How
> > > will this affect the numbers tracked by the persistent GCTracer?
> > 
> > This patch shouldn't change the numbers, unless I missed something. The new
> > start
> > and stop methods implement the functionality previously found in the
> > constructor and destructor of GCTracer. The incremental marking step
counters
> > are now
> > tracked directly in GCTracer, instead of copying them from
IncrementalMarking
> in
> > the constructor.
> 
> Yes, the mechanics of which numbers are gathered where didn't change. But the
> change in line 1107 of heap.cc no longer installs a different GCTracer object
> for the scavenge operation. Instead the GCTracer object for a mark-and-compact
> GC is shared with the one for a scavenge operation. So the statistics gathered
> while scavenging are gathered on top of any running mark-and-compact
operation.

So many comments already... I just had a brief look, a more detail review is in
progress.
I think this goes in the right direction. We should unify the statistics as much
as possible.
However, some of the numbers which are currently recorded for example during
incremental
marking maybe should not live in the tracer. Maybe we just want to use if for
measuring
execution time of gc operations?

[Hannes Payer (out of office), 2014-07-14 15:04:39]

I think the problem with the tracer is that it holds the state of three
different gc subsystems. This is a mess. Maybe we should have three tracer
instances: scavenger, mark-compact, and marking-steps. WDYT? Based on that we
can also keep a history of events for each subsystem and can simply query the
given instance.

[ernstm, 2014-07-16 07:47:27]

On 2014/07/14 15:04:39, Hannes Payer wrote:
> I think the problem with the tracer is that it holds the state of three
> different gc subsystems. This is a mess. Maybe we should have three tracer
> instances: scavenger, mark-compact, and marking-steps. WDYT? Based on that we
> can also keep a history of events for each subsystem and can simply query the
> given instance.

Per offline discussion, we want to separate tracing of scavenger, mark-compact,
and marking-steps. We also want to make sure that stats use purely for debugging
and profiling are separated from what we use to drive our heuristics (without
duplicating code or measurements). I'll close this CL now and start a new one,
cleaning up the existing GCTracer as a first step.