Add about:memory support for Linux....

Add about:memory support for Linux.
Also, refactors code in MemoryDetails and adds
ProcessMetrics::GetCommittedKBytes for Linux.

BUG=9653

[Evan Martin, 2009-08-03 05:16:11]

I think Dean or Joel should review the memory bits

http://codereview.chromium.org/159777/diff/1/3
File base/process_util_linux.cc (right):

http://codereview.chromium.org/159777/diff/1/3#newcode335
Line 335: size_t private_size = 0;
you can declare these variables closer to where they're used...

http://codereview.chromium.org/159777/diff/1/3#newcode336
Line 336: if (!file_util::ReadFileToString(stat_file, &maps))
should this error case fill in |usage| since the caller can't otherwise tell the
call failed?

http://codereview.chromium.org/159777/diff/1/3#newcode356
Line 356: unsigned int sep_pos = fields[0].find('-');
string::size_type here, i guess

http://codereview.chromium.org/159777/diff/1/3#newcode368
Line 368: // [r-][w-][x-][ps]
I think you have regex syntax here, right?  might make it more clear to sayu

"Second field is permissions formatted as four characters:"

http://codereview.chromium.org/159777/diff/1/6
File chrome/browser/browser_about_handler.cc (right):

http://codereview.chromium.org/159777/diff/1/6#newcode517
Line 517: for (unsigned int index = 0; index < browser_processes.size();
index++) {
size_t

http://codereview.chromium.org/159777/diff/1/7
File chrome/browser/memory_details.cc (right):

http://codereview.chromium.org/159777/diff/1/7#newcode182
Line 182: default:
Should this have a NOTREACHED() here?

http://codereview.chromium.org/159777/diff/1/4
File chrome/browser/memory_details_linux.cc (right):

http://codereview.chromium.org/159777/diff/1/4#newcode32
Line 32: l10n_util::GetString(IDS_PRODUCT_NAME);
We avoid statics as a rule; is there a reason for this one?

http://codereview.chromium.org/159777/diff/1/4#newcode33
Line 33: ProcessData g_process_template[MAX_BROWSERS] = {
Does this shadow the other definition?  I am confused by it :)

http://codereview.chromium.org/159777/diff/1/4#newcode40
Line 40: for (unsigned int index = 0; index < arraysize(g_process_template);
++index) {
size_t

http://codereview.chromium.org/159777/diff/1/4#newcode57
Line 57: for (unsigned int index = 0; index < arraysize(g_process_template);
index++) {
size_t

[Dean McNamee, 2009-08-03 15:30:15]

Adding Joel, who did the original work on this.

[Joel Stanley (old), 2009-08-03 16:02:37]

http://codereview.chromium.org/159777/diff/1/3
File base/process_util_linux.cc (right):

http://codereview.chromium.org/159777/diff/1/3#newcode331
Line 331: FilePath("/proc").Append(IntToString(process_)).Append("maps");
The other functions that scrape /proc/<pid> use smaps instead of maps, do you
have a reason to not be consistent?

smaps has the same data; the value 'Size' is the same as what you calculate for
size_t size = address_end - address_begin.

http://codereview.chromium.org/159777/diff/1/3#newcode442
Line 442: ws_usage->shareable = ws_usage->shared;
from chrome/browser/resources/about_memory.html, which will be the only consumer
of this data on linux:

> tot_ws_priv = ws_priv + ws_shareable - ws_shared

Ok, so on linux total private will be private. Makes sense

> ws_priv + ws_shareable - ws_shared + (ws_shared / processes)

this looks like windows trying to calculate 'PSS' to me.

> ws_priv + ws_shareable

Can't work out what this is displayed as, so not sure if it makes sense.

If it were up to me, we would stop trying to fit the square linux memory stats
peg into the round windows memory stats hole.  Linux should parse
/proc/<pid>/smaps once, parse /proc/<pid>/status once, and the 5 columns on
about:memory will read:

Memory:
Private = Private_Dirty + Private_Clean
Shared = Pss - Private
Total = sum of the two above

Virtual Memory:
Private = VmSize
Mapped = VmRSS

A function in process_util can calculate these numbers directly, instead of
having the javascript do this.  The data structure for holding the info can
represent what is to be shown on the screen, instead of what the Windows API
returns.

http://codereview.chromium.org/159777/diff/1/7
File chrome/browser/memory_details.cc (right):

http://codereview.chromium.org/159777/diff/1/7#newcode182
Line 182: default:
Where does the zgyote fall into this?

http://codereview.chromium.org/159777/diff/1/4
File chrome/browser/memory_details_linux.cc (right):

http://codereview.chromium.org/159777/diff/1/4#newcode35
Line 35: { L"Firefox", L"firefox-bin", },
Some other browsers present on my Ubuntu Karmic system:
 - Firefox 3.5 is 'firefox-3.5'
 - Firefox 3 is 'firefox'
 - Epiphany is 'epiphany-browser'
 - Midori is 'midori'

[Joel Stanley (old), 2009-08-03 16:11:54]

On 2009/08/03 16:02:37, Joel Stanley wrote:
> If it were up to me, we would stop trying to fit the square linux memory stats
> peg into the round windows memory stats hole.

To reiterate, the stats we have now are sort of "How the NT kernel happens to
measure memory" and Linux does not fit neatly into that model.  To make this
work on Linux (and potentially mac), this part of the code needs to be
refactored to be cross platform; it the code in base/ should expose to chrome
the stats chrome needs, not the stats the OS presents.

[Anand K. Mistry, 2009-08-05 03:28:45]

On 2009/08/03 16:11:54, Joel Stanley wrote:
> On 2009/08/03 16:02:37, Joel Stanley wrote:
> > If it were up to me, we would stop trying to fit the square linux memory
stats
> > peg into the round windows memory stats hole.
> 
> To reiterate, the stats we have now are sort of "How the NT kernel happens to
> measure memory" and Linux does not fit neatly into that model.  To make this
> work on Linux (and potentially mac), this part of the code needs to be
> refactored to be cross platform; it the code in base/ should expose to chrome
> the stats chrome needs, not the stats the OS presents.

I'm inclined to agree, so I'm gonna do some more refactoring before I post a
second revision.

[viettrungluu-gmail, 2009-08-24 02:44:16]

On 2009/08/05 03:28:45, Anand K. Mistry wrote:
> I'm inclined to agree, so I'm gonna do some more refactoring before I post a
> second revision.

For xref-ing purposes, here's a patch for the Mac side (not yet ready for review
-- waiting on your refactoring, which I think is a great idea):

http://codereview.chromium.org/173261