Provide access to process private and shared memory size from WebKit code

Provide access to process private and shared memory size from WebKit code. We need this to show render process native memory distribution between different components in the DevTools. WebKit part of the change was landed as part of https://bugs.webkit.org/show_bug.cgi?id=87263


BUG=108777
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=139755

[pfeldman, 2012-03-11 15:25:26]

lgtm

http://codereview.chromium.org/9669039/diff/4/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9669039/diff/4/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:686: {
{ on wrong line.

[yurys, 2012-03-11 15:25:58]

http://codereview.chromium.org/9669039/diff/4/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9669039/diff/4/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:686: {
On 2012/03/11 15:25:26, pfeldman wrote:
> { on wrong line.

Done.

[yurys, 2012-03-11 15:46:16]

Darin, I need an owner's stamp, could you take a look?

[yurys, 2012-05-29 08:07:44]

Darin or Tony can you take a look at this change?

[tony, 2012-05-29 18:11:18]

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:688: static ProcessMetrics*
process_metrics =
CR_DEFINE_STATIC_LOCAL.

It would be nice if we could share the ProcessMetrics instance with
getMemoryUsageMB.

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:694: static ProcessMetrics*
process_metrics =
CR_DEFINE_STATIC_LOCAL

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.h (right):

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.h:64: size_t* shared_bytes);
Some comments about the upstream change:
Should this method have been added next to the memory functions in
Source/Platform/chromium/public/Platform.h?  Also, webkit style doesn't normally
prefix methods with 'get'.  Finally, I would have included the units in the
method name, e.g., processMemorySizesInBytes.

[tony, 2012-05-29 18:12:42]

To clarify:
It's fine to ignore my comments about the upstream change (they're just nits),
but I think we should definitely share ProcessMetrics with the other code rather
than created two separate instances.

[yurys, 2012-05-30 08:06:27]

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:688: static ProcessMetrics*
process_metrics =
On 2012/05/29 18:11:18, tony wrote:
> CR_DEFINE_STATIC_LOCAL.
> 
CR_DEFINE_STATIC_LOCAL cannot be used here as it calls new on the first argument
while we call a factory method here.


> It would be nice if we could share the ProcessMetrics instance with
> getMemoryUsageMB.
Done.

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.cc:694: static ProcessMetrics*
process_metrics =
On 2012/05/29 18:11:18, tony wrote:
> CR_DEFINE_STATIC_LOCAL
See my previous comment.

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
File webkit/glue/webkitplatformsupport_impl.h (right):

http://codereview.chromium.org/9669039/diff/5/webkit/glue/webkitplatformsuppo...
webkit/glue/webkitplatformsupport_impl.h:64: size_t* shared_bytes);
On 2012/05/29 18:11:18, tony wrote:
> Some comments about the upstream change:
> Should this method have been added next to the memory functions in
> Source/Platform/chromium/public/Platform.h?
This patch was created when  Source/Platform/chromium/public/Platform.h didn't
exist (2.5 months ago).

> Also, webkit style doesn't normally prefix methods with 'get'.
> Finally, I would have included the units in the
> method name, e.g., processMemorySizesInBytes.
Some methods returning results in arguments have 'get' prefix and I followed
that naming style. I've sent WebKit change addressing your comments:
https://bugs.webkit.org/show_bug.cgi?id=87830

[yurys, 2012-05-30 08:06:47]

On 2012/05/29 18:12:42, tony wrote:
> To clarify:
> It's fine to ignore my comments about the upstream change (they're just nits),
> but I think we should definitely share ProcessMetrics with the other code
rather
> than created two separate instances.
Fixed.

[tony, 2012-05-30 17:09:03]

LGTM!

[ulan, 2012-09-21 13:30:08]

Yury, quick question regarding this CL.

https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
File webkit/glue/webkitplatformsupport_impl.cc (left):

https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
webkit/glue/webkitplatformsupport_impl.cc:696: return
process_metrics->GetWorkingSetSize() >> 20;
Just noticed that this is missing in the current ToT.

Was this removed intentionally? 

GetPagefileUsage() returns virtual memory size and therefore greatly
overestimates real memory usage. This causes WebKit to send bogus low memory
notifications to V8 and leads to big GC pauses. It is better to use
GetWorkingSetSize().

[yurys, 2012-09-21 13:52:11]

https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
File webkit/glue/webkitplatformsupport_impl.cc (left):

https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
webkit/glue/webkitplatformsupport_impl.cc:696: return
process_metrics->GetWorkingSetSize() >> 20;
On 2012/09/21 13:30:08, ulan wrote:
> Just noticed that this is missing in the current ToT.
> 
> Was this removed intentionally? 
> 
> GetPagefileUsage() returns virtual memory size and therefore greatly
> overestimates real memory usage. This causes WebKit to send bogus low memory
> notifications to V8 and leads to big GC pauses. It is better to use
> GetWorkingSetSize(). 

Looks like GetWorkingSetSize was deleted by mistake when I extracted
CurrentProcessMetrics function. We should return it back.

[ulan, 2012-09-21 13:57:02]

On 2012/09/21 13:52:11, Yury Semikhatsky wrote:
>
https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
> File webkit/glue/webkitplatformsupport_impl.cc (left):
> 
>
https://chromiumcodereview.appspot.com/9669039/diff/9002/webkit/glue/webkitpl...
> webkit/glue/webkitplatformsupport_impl.cc:696: return
> process_metrics->GetWorkingSetSize() >> 20;
> On 2012/09/21 13:30:08, ulan wrote:
> > Just noticed that this is missing in the current ToT.
> > 
> > Was this removed intentionally? 
> > 
> > GetPagefileUsage() returns virtual memory size and therefore greatly
> > overestimates real memory usage. This causes WebKit to send bogus low memory
> > notifications to V8 and leads to big GC pauses. It is better to use
> > GetWorkingSetSize(). 
> 
> Looks like GetWorkingSetSize was deleted by mistake when I extracted
> CurrentProcessMetrics function. We should return it back.

Ah, I see. Thank you for the quick reply. :)
I'll upload CL that restores it.