Adding flag to specify the used memory pressure strategy

Adding flag to specify the used memory pressure strategy

This CL adds a flag to change the memory pressure event thresholds. This flag will later on then used in a finch experiment to test the influence of the thresholds on the system.

BUG=449374
TEST=-

Committed: https://crrev.com/f6f74b0eeeb0598b982f44f663405fb9ca919907
Cr-Commit-Position: refs/heads/master@{#312499}

[Mr4D (OOO till 08-26), 2015-01-21 00:40:47]

skuhne@chromium.org changed reviewers:
+ creis@chromium.org

[Mr4D (OOO till 08-26), 2015-01-21 00:40:59]

Please have a look!

[Charlie Reis, 2015-01-21 17:42:06]

No objections here to having a way to experiment with this, though I'm also not
super familiar with this code.  I'd imagine that jamescook@ might be a more
knowledgeable reviewer here if you want one, given his tab discard work.

At any rate, just a few nits and I think it's fine.

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
File base/chromeos/memory_pressure_observer_chromeos.cc (right):

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
base/chromeos/memory_pressure_observer_chromeos.cc:50: (thresholds ==
AGGRESSIVE_MODERATE_THRESHOLD ||
Might this be easier to read with helper functions, like
IsModerateThresholdAggressive(thresholds) or even
GetModerateThreshold(thresholds)?

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
File base/chromeos/memory_pressure_observer_chromeos.h (right):

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
base/chromeos/memory_pressure_observer_chromeos.h:31: DEFAULT_THRESHOLDS = 0,   
         // Use the default.
These names and comments are confusing to me.  Normal, aggressive, moderate, and
critical could all be names on the same scale, so please add a comment
explaining how they relate.

Also, I'm not sure about the conventions in this code, but the content module
tends to name enums differently:
http://www.chromium.org/developers/content-module/content-api
"enum values should start with the name of the type, i.e. PAGE_TRANSITION_LINK
in the content::PageTransition enum"

https://codereview.chromium.org/803443006/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/803443006/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:1136: Release strategy for advanced memory
pressure handling
"Release strategy" seems ambiguous to me (vs Chrome releases).

[Mr4D (OOO till 08-26), 2015-01-21 18:52:13]

skuhne@chromium.org changed reviewers:
+ jamescook@chromium.org

[Mr4D (OOO till 08-26), 2015-01-21 18:52:14]

Addressed! Please have another look!

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
File base/chromeos/memory_pressure_observer_chromeos.cc (right):

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
base/chromeos/memory_pressure_observer_chromeos.cc:50: (thresholds ==
AGGRESSIVE_MODERATE_THRESHOLD ||
On 2015/01/21 17:42:05, Charlie Reis wrote:
> Might this be easier to read with helper functions, like
> IsModerateThresholdAggressive(thresholds) or even
> GetModerateThreshold(thresholds)?

Done.

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
File base/chromeos/memory_pressure_observer_chromeos.h (right):

https://codereview.chromium.org/803443006/diff/1/base/chromeos/memory_pressur...
base/chromeos/memory_pressure_observer_chromeos.h:31: DEFAULT_THRESHOLDS = 0,   
         // Use the default.
On 2015/01/21 17:42:05, Charlie Reis wrote:
> These names and comments are confusing to me.  Normal, aggressive, moderate,
and
> critical could all be names on the same scale, so please add a comment
> explaining how they relate.
> 
> Also, I'm not sure about the conventions in this code, but the content module
> tends to name enums differently:
> http://www.chromium.org/developers/content-module/content-api
> "enum values should start with the name of the type, i.e. PAGE_TRANSITION_LINK
> in the content::PageTransition enum"

Done.

https://codereview.chromium.org/803443006/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/803443006/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:1136: Release strategy for advanced memory
pressure handling
On 2015/01/21 17:42:06, Charlie Reis wrote:
> "Release strategy" seems ambiguous to me (vs Chrome releases).

Done.

[Charlie Reis, 2015-01-21 19:37:55]

Thanks, that's much easier to follow.  LGTM with nit.

https://codereview.chromium.org/803443006/diff/20001/chromeos/chromeos_switch...
File chromeos/chromeos_switches.cc (right):

https://codereview.chromium.org/803443006/diff/20001/chromeos/chromeos_switch...
chromeos/chromeos_switches.cc:309: if (option == "1")
nit: These all need braces because the body doesn't fit on one line.  (Sorry for
missing this earlier.)

[Mr4D (OOO till 08-26), 2015-01-21 21:01:36]

Many thanks!

https://codereview.chromium.org/803443006/diff/20001/chromeos/chromeos_switch...
File chromeos/chromeos_switches.cc (right):

https://codereview.chromium.org/803443006/diff/20001/chromeos/chromeos_switch...
chromeos/chromeos_switches.cc:309: if (option == "1")
On 2015/01/21 19:37:55, Charlie Reis wrote:
> nit: These all need braces because the body doesn't fit on one line.  (Sorry
for
> missing this earlier.)

Done.

[James Cook, 2015-01-21 21:24:26]

LGTM with two very optional nits

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
File base/chromeos/memory_pressure_observer_chromeos.cc (right):

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
base/chromeos/memory_pressure_observer_chromeos.cc:37:
MEMORY_PRESSURE_THRESHOLD_AGGRESSIVE_CACHE_DISCARD ||
nit: Given that these are namespaced with MemoryPressureObserverChromeOS you
could probably just call them THRESHOLD_*, which would make them shorter. Or
just keep them they way they are.  I don't feel strongly about it.

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
File base/chromeos/memory_pressure_observer_chromeos.h (right):

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
base/chromeos/memory_pressure_observer_chromeos.h:94: // The thresholds for
moderate and critical pressure.
nit: What are the units? Percent?

[Mr4D (OOO till 08-26), 2015-01-21 21:54:34]

Addressed! Thanks for your review!

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
File base/chromeos/memory_pressure_observer_chromeos.cc (right):

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
base/chromeos/memory_pressure_observer_chromeos.cc:37:
MEMORY_PRESSURE_THRESHOLD_AGGRESSIVE_CACHE_DISCARD ||
On 2015/01/21 21:24:25, James Cook wrote:
> nit: Given that these are namespaced with MemoryPressureObserverChromeOS you
> could probably just call them THRESHOLD_*, which would make them shorter. Or
> just keep them they way they are.  I don't feel strongly about it.

Done.

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
File base/chromeos/memory_pressure_observer_chromeos.h (right):

https://codereview.chromium.org/803443006/diff/60001/base/chromeos/memory_pre...
base/chromeos/memory_pressure_observer_chromeos.h:94: // The thresholds for
moderate and critical pressure.
On 2015/01/21 21:24:25, James Cook wrote:
> nit: What are the units? Percent?

Done.

[Mr4D (OOO till 08-26), 2015-01-21 21:54:37]

The CQ bit was checked by skuhne@chromium.org

[Mr4D (OOO till 08-26), 2015-01-21 21:55:59]

The CQ bit was unchecked by skuhne@chromium.org

[Mr4D (OOO till 08-26), 2015-01-21 22:02:38]

skuhne@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Mr4D (OOO till 08-26), 2015-01-21 22:02:39]

Could you please do an owners review for histograms.xls?

Many thanks!

[Alexei Svitkine (slow), 2015-01-21 22:04:36]

lgtm

[Mr4D (OOO till 08-26), 2015-01-21 22:08:09]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2015-01-21 22:09:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/803443006/100001

[commit-bot: I haz the power, 2015-01-22 00:06:11]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-01-22 00:07:05]

Patchset 6 (id:??) landed as
https://crrev.com/f6f74b0eeeb0598b982f44f663405fb9ca919907
Cr-Commit-Position: refs/heads/master@{#312499}