Add histograms to track fraction of heap spaces and percentage of crankshaft code.

Add histograms to track fraction of heap spaces and percentage of generated crankshaft code.

BUG=None
R=mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=17198

[rmcilroy, 2013-10-11 16:24:15]

PTAL.

[Michael Starzinger, 2013-10-14 09:18:37]

One central comment about the way the accounting is done.

https://codereview.chromium.org/27023003/diff/3001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/27023003/diff/3001/src/heap.h#newcode2374
src/heap.h:2374: intptr_t full_codegen_bytes_;
nit: I know the existing code is not consistent about that at all, but I think
these should be size_t here.

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

https://codereview.chromium.org/27023003/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1010: isolate_->heap()->DecrementCodeGeneratedBytes(
IIUC, this tries to keep the new code-bytes counters in the heap accurate.
Unfortunately this is not possible as code can just become unreachable (e.g.
closure becomes unreachable, whole context becomes unreachable). In general
there is no place that visits "dead code" before it's memory is reclaimed. The
flushed code is just a subset of the code being reclaimed.

We should clarify what this new counter actually wants to represent. If we
really need a fully accurate number, one accurate way to track it would be to
reset it to zero before marking starts and then let the tracer count anew for
every GC cycle. But that is hard to get right, especially in light of
incremental marking, where the same object might be visited more than once.

I would split this part out in a separate CL, as it is not entirely trivial to
get this accounting right.

[rmcilroy, 2013-10-14 11:24:14]

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

https://codereview.chromium.org/27023003/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1010: isolate_->heap()->DecrementCodeGeneratedBytes(
On 2013/10/14 09:18:37, Michael Starzinger wrote:
> IIUC, this tries to keep the new code-bytes counters in the heap accurate.
> Unfortunately this is not possible as code can just become unreachable (e.g.
> closure becomes unreachable, whole context becomes unreachable). In general
> there is no place that visits "dead code" before it's memory is reclaimed. The
> flushed code is just a subset of the code being reclaimed.
> 
> We should clarify what this new counter actually wants to represent. If we
> really need a fully accurate number, one accurate way to track it would be to
> reset it to zero before marking starts and then let the tracer count anew for
> every GC cycle. But that is hard to get right, especially in light of
> incremental marking, where the same object might be visited more than once.
> 
> I would split this part out in a separate CL, as it is not entirely trivial to
> get this accounting right.

Ahh right - I thought code would be added to this candidate list when
contexts/closures went out of scope, but what you say makes sense. I've updated
the CL to make this counter a measure of the fraction of crankshafted v.s.
non-crankshafted code that is generated overall (not the amount live in the
heap).  This should be accurate enough.

[Michael Starzinger, 2013-10-14 13:12:56]

LGTM with one suggestion. Will land this for you once it has been addressed.

https://codereview.chromium.org/27023003/diff/9001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/27023003/diff/9001/src/heap.h#newcode2366
src/heap.h:2366: size_t full_codegen_bytes_generated_;
Let's add a short one-liner comment here saying that these counters are
monotonically increasing and never reset.

[rmcilroy, 2013-10-14 13:29:52]

Thanks Michael!

https://codereview.chromium.org/27023003/diff/3001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/27023003/diff/3001/src/heap.h#newcode2374
src/heap.h:2374: intptr_t full_codegen_bytes_;
On 2013/10/14 09:18:37, Michael Starzinger wrote:
> nit: I know the existing code is not consistent about that at all, but I think
> these should be size_t here.

Done.

https://codereview.chromium.org/27023003/diff/9001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/27023003/diff/9001/src/heap.h#newcode2366
src/heap.h:2366: size_t full_codegen_bytes_generated_;
On 2013/10/14 13:12:57, Michael Starzinger wrote:
> Let's add a short one-liner comment here saying that these counters are
> monotonically increasing and never reset.

Done.

[Michael Starzinger, 2013-10-14 14:00:37]

Committed patchset #4 manually as r17198 (presubmit successful).