Add histograms for total allocated/live heap size, as well as allocated size and percentage of tota…

Add histograms for total allocated/live heap size, as well as allocated size and percentage of total for map and cell

BUG=none
TEST=none
R=mstarzinger@chromium.org

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

[jochen (gone - plz use gerrit), 2012-08-08 14:43:53]

plz review

[Michael Starzinger, 2012-08-08 18:44:36]

The main comment is about the names. The rest are just observations.

https://chromiumcodereview.appspot.com/10854043/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/heap.cc#newcode464
src/heap.cc:464:
isolate_->counters()->total_heap_size()->AddSample(CommittedMemory() / KB);
It doesn't make much of a difference, but we could sample these last four
numbers even if CommittedMemory() == 0.

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.cc
File src/v8-counters.cc (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.cc#new...
src/v8-counters.cc:49: Histogram name = { #caption, 1000, 500000, 50, NULL,
false }; \
This seems to set the maximum value for memory histograms to ~500MB, the total
heap size can easily exceed that. On ia32 we limit the old generation to 700MB,
on x64 to 1400MB. I'm not that familiar with how the histograms work and if that
poses a problem, just wanted to point it out.

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.h
File src/v8-counters.h (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.h#newc...
src/v8-counters.h:79: HM(total_cell_space_size, V8.MemoryCellSpaceSize)
We should start using consistent names for the heap-related counters and
histograms, otherwise this will get out of hand. Especially when it comes to
"live" vs. "used" and "total" vs. "committed". How about the following names for
the new histograms? WDYT?

- heap_fraction_map_space
- heap_fraction_cell_space
- heap_sample_[total_]committed  (the "total_" could be omitted)
- heap_sample_[total_]used       (likewise)
- heap_sample_map_space_committed
- heap_sample_cell_space_committed

I have no strong opinion about the externally visible names, so we can leave
them as they are.

[jochen (gone - plz use gerrit), 2012-08-09 13:31:29]

https://chromiumcodereview.appspot.com/10854043/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/heap.cc#newcode464
src/heap.cc:464:
isolate_->counters()->total_heap_size()->AddSample(CommittedMemory() / KB);
On 2012/08/08 18:44:36, Michael Starzinger wrote:
> It doesn't make much of a difference, but we could sample these last four
> numbers even if CommittedMemory() == 0.

We'd just keep adding 0 samples (probably during startup or so)

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.cc
File src/v8-counters.cc (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.cc#new...
src/v8-counters.cc:49: Histogram name = { #caption, 1000, 500000, 50, NULL,
false }; \
On 2012/08/08 18:44:36, Michael Starzinger wrote:
> This seems to set the maximum value for memory histograms to ~500MB, the total
> heap size can easily exceed that. On ia32 we limit the old generation to
700MB,
> on x64 to 1400MB. I'm not that familiar with how the histograms work and if
that
> poses a problem, just wanted to point it out.

It's the usual histogram layout for KB values (see base/metrics/histogram.h).
The upper bound of the last bucket is INT_MAX

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.h
File src/v8-counters.h (right):

https://chromiumcodereview.appspot.com/10854043/diff/1/src/v8-counters.h#newc...
src/v8-counters.h:79: HM(total_cell_space_size, V8.MemoryCellSpaceSize)
On 2012/08/08 18:44:36, Michael Starzinger wrote:
> We should start using consistent names for the heap-related counters and
> histograms, otherwise this will get out of hand. Especially when it comes to
> "live" vs. "used" and "total" vs. "committed". How about the following names
for
> the new histograms? WDYT?
> 
> - heap_fraction_map_space
> - heap_fraction_cell_space
> - heap_sample_[total_]committed  (the "total_" could be omitted)
> - heap_sample_[total_]used       (likewise)
> - heap_sample_map_space_committed
> - heap_sample_cell_space_committed
> 
> I have no strong opinion about the externally visible names, so we can leave
> them as they are.

Done.

[Michael Starzinger, 2012-08-10 09:52:59]

LGTM. Will land that for you.