cros: Log per-process memory use on low memory events

cros: Log per-process memory use on low memory events

We're seeing a lot of tab reloads caused by the tab discarder, but we're not sure yet where the memory is going.  Dump the per-process private and shared memory use (same as the Task Manager columns) for Chrome processes when we hit low memory, such that we can see it in the system logs when users send us a feedback report.  This adds tens of milliseconds to the delay between the low memory signal and the tab discard starting, but that isn't significant as we allow 750 ms for the discard to happen and the memory stats are collected asynchronously off of the UI thread.

Also refactor when we log UMA memory statistics - we used to do it every time we collected memory details, but with this change we need to be able to collect the details and skip additional UMA stats.

BUG=124038
TEST=browser_tests OomPriorityManagerTest, manual inspection of logs after triggering tab discard with about:discards


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133232

[James Cook, 2012-04-20 00:05:58]

Greg, PTAL.

James, I need OWNERS approval for chrome/browser/ui/webui/about_ui.cc -- I
changed the signature for a command I can trigger from an about: page.

[Greg Spencer (Chromium), 2012-04-20 00:28:21]

LGTM w/nit

http://codereview.chromium.org/10151005/diff/1/chrome/browser/automation/auto...
File chrome/browser/automation/automation_provider_observers.cc (right):

http://codereview.chromium.org/10151005/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_provider_observers.cc:2722: // tests, but
it's how the system has always worked.
Can you elaborate on what "this" and "it's" refer to here? (I mean, I know
"this" refers to the function call, but what does the function call imply?)
Otherwise I'm unclear why you added it if you put in a TODO to remove it.

http://codereview.chromium.org/10151005/diff/1/chrome/browser/oom_priority_ma...
File chrome/browser/oom_priority_manager.cc (right):

http://codereview.chromium.org/10151005/diff/1/chrome/browser/oom_priority_ma...
chrome/browser/oom_priority_manager.cc:112: delete this;
Interesting solution.  I like it.

[jar (doing other things), 2012-04-20 02:19:18]

Briefly (insufficiently?) scanning this, I was a bit concerned that we'd be
changing when and how often we recorded memory details. If we do, then we'd bias
the samples potentially toward getting more samples in a low memory situation.

Can you calm my fears by at least arguing that this can only have an impact on
CrOS stats?  ...and then, perchance completely allay my fears by mentioning how
it won't bias CrOS stats towards getting "more samples" from a troubled user?

Thanks,

Jim

[do not use, 2012-04-20 02:45:20]

> Can you calm my fears by at least arguing that this can only have an impact on
> CrOS stats?  ...and then, perchance completely allay my fears by mentioning
how
> it won't bias CrOS stats towards getting "more samples" from a troubled user?

It won't collect more stats from a troubled user.  I intentionally changed
MemoryDetails to make the update of UMA memory histograms optional, and I don't
update the histograms when it is called from the tab discard code.

This also revealed a couple of other cases where it's debatable whether or not
we should record histogram data, but I elected to leave them as-is so I wouldn't
perturb the stats.  For example, we record additional memory samples every time
a user opens about:memory, which seems undesirable, although rare enough it
probably doesn't change the collective stats.

Finally, the low-memory monitoring is Chrome OS only, so other platforms are
safe even if I screwed something up.

[do not use, 2012-04-20 02:46:59]

Ah, perhaps my code is less clear than it could be.  I'll make it more obvious
by making MemoryDetails::StartFetch() take a parameter indicating whether or not
histograms will be updated.

[James Cook, 2012-04-20 04:59:39]

Updated - is this more clear?

[Greg Spencer (Chromium), 2012-04-20 16:00:01]

Looks clearer to me.

[James Hawkins, 2012-04-20 16:28:43]

webui lgtm

[commit-bot: I haz the power, 2012-04-20 17:04:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamescook@chromium.org/10151005/23001

[commit-bot: I haz the power, 2012-04-20 18:59:06]

Change committed as 133232