Implement committed physical memory stats for Linux.

Implement committed physical memory stats for Linux.

The patch introduces CommittedPhysicalMemory function to the Heap class
that reports committed *physical* memory acquired from the OS.
It is important because some OSes may postpone actual commitment on e.g.
first access to the previously committed region.
So reporting just plain committed size led to various weird artifacts
like DevTools showing V8 allocated memory higher than the whole process
private size.

BUG=v8:2191
Committed: https://code.google.com/p/v8/source/detail?r=12625

[Vyacheslav Egorov (Google), 2012-09-21 14:29:06]

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.cc#newcode215
src/heap.cc:215: size_t Heap::CommittedPhysicalMemory() {
This function might still return a number that is not accurate. Because when you
iterate over spaces to discover V8 Pages you touch memory and thus affect its
state. You might also end up asking OS to "unswap" OS-pages (ones that back
V8-Page headers that you access) that might have been swapped out and thus end
up with a bigger RSS than RSS before you called this function.

Also consider the following case:

1) you compute CommittedPhysicalMemory for new space
2) start computing CommittedPhysicalMemory, touch some swapped pages -> OS
un-swap's them and swaps pages backing new space this makes results computed for
new space obsolete.

[alph-g, 2012-09-21 14:40:55]

On 2012/09/21 14:29:06, Vyacheslav Egorov (Google) wrote:
> https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.cc
> File src/heap.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.cc#newcode215
> src/heap.cc:215: size_t Heap::CommittedPhysicalMemory() {
> This function might still return a number that is not accurate. Because when
you
> iterate over spaces to discover V8 Pages you touch memory and thus affect its
> state. You might also end up asking OS to "unswap" OS-pages (ones that back
> V8-Page headers that you access) that might have been swapped out and thus end
> up with a bigger RSS than RSS before you called this function.
> 
> Also consider the following case:
> 
> 1) you compute CommittedPhysicalMemory for new space
> 2) start computing CommittedPhysicalMemory, touch some swapped pages -> OS
> un-swap's them and swaps pages backing new space this makes results computed
for
> new space obsolete.

Yep. I realize that all these memory calculations are approximate. Obviously
mincore syscall makes a snapshot which may be completely wrong right at the
point it returns. However this approximation is much better than the one where
we just take a committed size. I can see on our chart in DevTools.

[Yang, 2012-09-24 14:05:35]

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.h
File src/heap.h (right):

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.h#newcode490
src/heap.h:490: size_t CommittedPhysicalMemory();
Any reason why this is a size_t while the rest are intptr_t?

[alph-g, 2012-09-24 14:09:59]

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.h
File src/heap.h (right):

https://chromiumcodereview.appspot.com/10961042/diff/2001/src/heap.h#newcode490
src/heap.h:490: size_t CommittedPhysicalMemory();
On 2012/09/24 14:05:35, Yang wrote:
> Any reason why this is a size_t while the rest are intptr_t?

I have no idea why the rest are intptr_t. The returned value is obviously
size_t, it cannot be negative.

[Michael Starzinger, 2012-09-27 09:46:15]

One additional round of comments.

https://codereview.chromium.org/10961042/diff/2001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/10961042/diff/2001/include/v8.h#newcode2769
include/v8.h:2769: size_t committed_physical_size() { return
committed_physical_size_; }
Can we rename this to "total_physical_size"? This makes more sense in the API
naming scheme, we never use the word "committed" there AFAIK. I am totally fine
with the internal name.

https://codereview.chromium.org/10961042/diff/2001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/10961042/diff/2001/src/heap.h#newcode490
src/heap.h:490: size_t CommittedPhysicalMemory();
On 2012/09/24 14:09:59, alph-g wrote:
> On 2012/09/24 14:05:35, Yang wrote:
> > Any reason why this is a size_t while the rest are intptr_t?
> 
> I have no idea why the rest are intptr_t. The returned value is obviously
> size_t, it cannot be negative.

Yeah, the others should be size_t as well I guess. But that is for another day
to cleanup.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode492
src/spaces.cc:492: size_t size = area_end_ - area_start_;
Can we use area_size() for this calculation?

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode1412
src/spaces.cc:1412: NewSpacePage* page = anchor_.next_page();
Can we use NewSpacePageIterator for this iteration? It should work on just one
of the two SemiSpaces as well.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode2731
src/spaces.cc:2731: while (current != NULL) {
Can we use LargeObjectIterator for this iteration?

[alph, 2012-09-27 12:51:13]

PTAL

https://codereview.chromium.org/10961042/diff/2001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/10961042/diff/2001/include/v8.h#newcode2769
include/v8.h:2769: size_t committed_physical_size() { return
committed_physical_size_; }
On 2012/09/27 09:46:16, Michael Starzinger wrote:
> Can we rename this to "total_physical_size"? This makes more sense in the API
> naming scheme, we never use the word "committed" there AFAIK. I am totally
fine
> with the internal name.

Done.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode492
src/spaces.cc:492: size_t size = area_end_ - area_start_;
On 2012/09/27 09:46:16, Michael Starzinger wrote:
> Can we use area_size() for this calculation?
Let's try. Hopefully it won't go beyond 2GB limit.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode1412
src/spaces.cc:1412: NewSpacePage* page = anchor_.next_page();
On 2012/09/27 09:46:16, Michael Starzinger wrote:
> Can we use NewSpacePageIterator for this iteration? It should work on just one
> of the two SemiSpaces as well.

Done.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode2731
src/spaces.cc:2731: while (current != NULL) {
On 2012/09/27 09:46:16, Michael Starzinger wrote:
> Can we use LargeObjectIterator for this iteration?

I'm not sure. It returns not a page, but a HeapObject from each page.

[Michael Starzinger, 2012-09-27 13:13:28]

LGTM. I'll land that for you.

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/10961042/diff/2001/src/spaces.cc#newcode2731
src/spaces.cc:2731: while (current != NULL) {
On 2012/09/27 12:51:13, alph wrote:
> On 2012/09/27 09:46:16, Michael Starzinger wrote:
> > Can we use LargeObjectIterator for this iteration?
> 
> I'm not sure. It returns not a page, but a HeapObject from each page.

You are right, my bad.