Add MemoryWedge to ios/.

Add MemoryWedge to ios/.

A MemoryWedge is a single chunk of memory allocated. One passes it a size and the wedge is added to the resident memory used by the current app.

Furthermore, it adds an experimental flag for iOS based on a variation param, to drive the size of the wedge that could be inserted for a given run.

BUG=TBD
R=stuartmorgan,asvitkine

Committed: https://crrev.com/68537275ee0c46477455e14127fd61c0988e60f9
Cr-Commit-Position: refs/heads/master@{#327471}

[lpromero, 2015-04-28 11:31:58]

The CQ bit was checked by lpromero@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-28 11:32:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1107963002/1

[commit-bot: I haz the power, 2015-04-28 11:32:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-28 11:32:05]

Dry run: No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Alexei Svitkine (slow), 2015-04-28 16:52:58]

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

[Alexei Svitkine (slow), 2015-04-28 16:52:58]

Looks good overall, a few style comments.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.h (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.h:18: unsigned MemoryWedgeSizeInMB();
Nit: I think size_t is preferred for unsigned values in Chromium code.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.mm (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:41:
base::StringToUint(wedge_size_string, &wedge_size_in_mb);
Should this check return value?

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags_unittest.mm (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags_unittest.mm:34: };
DISALLOW_COPY_AND_ASSIGN()

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge.cc (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.cc:11: namespace {
Nit: Add an empty line after and before the closing }

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.cc:26: if (wedge_size_in_mb == 0) {
Nit: No {}'s

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge.h (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.h:14: // For testing only.
Name it RemoveWedgeForTesting()?

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge_unittest.cc (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge_unittest.cc:44: };
DISALLOW_COPY_AND_ASSIGN()

[lpromero, 2015-04-28 18:12:46]

Addressed feeedback.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.h (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.h:18: unsigned MemoryWedgeSizeInMB();
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> Nit: I think size_t is preferred for unsigned values in Chromium code.

Switched to StringToSizeT internally, as you advised.
Done.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.mm (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:41:
base::StringToUint(wedge_size_string, &wedge_size_in_mb);
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> Should this check return value?

Discussed over IM.
Will check the bool returned and set to 0 in case of failure.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags_unittest.mm (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags_unittest.mm:34: };
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> DISALLOW_COPY_AND_ASSIGN()

Done.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge.cc (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.cc:11: namespace {
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> Nit: Add an empty line after and before the closing }

Done.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.cc:26: if (wedge_size_in_mb == 0) {
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> Nit: No {}'s

Done.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge.h (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge.h:14: // For testing only.
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> Name it RemoveWedgeForTesting()?

Done.

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge_unittest.cc (right):

https://codereview.chromium.org/1107963002/diff/20001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge_unittest.cc:44: };
On 2015/04/28 16:52:58, Alexei Svitkine wrote:
> DISALLOW_COPY_AND_ASSIGN()

Done.

[Alexei Svitkine (slow), 2015-04-28 18:43:40]

asvitkine@chromium.org changed reviewers:
- asvitkine@google.com

[Alexei Svitkine (slow), 2015-04-28 18:43:40]

lgtm % remaining comments

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.mm (right):

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:33: // Otherwise, get from Finch.
Nit: Finch is a codename, so we avoid using in code. Just say "from a variation
param.".

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:41: if
(base::StringToUint(wedge_size_string, &wedge_size_in_mb))
Change to StringToSizeT

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge_unittest.cc (right):

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge_unittest.cc:12: namespace {
Use namespace memory_wedge to match the cc file.

[lpromero, 2015-04-28 22:01:46]

Ping stuartmorgan. This is ready to be pushed.

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
File ios/chrome/browser/experimental_flags.mm (right):

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:33: // Otherwise, get from Finch.
On 2015/04/28 18:43:40, Alexei Svitkine wrote:
> Nit: Finch is a codename, so we avoid using in code. Just say "from a
variation
> param.".

Done.

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/expe...
ios/chrome/browser/experimental_flags.mm:41: if
(base::StringToUint(wedge_size_string, &wedge_size_in_mb))
On 2015/04/28 18:43:40, Alexei Svitkine wrote:
> Change to StringToSizeT

Done.

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/memo...
File ios/chrome/browser/memory/memory_wedge_unittest.cc (right):

https://codereview.chromium.org/1107963002/diff/40001/ios/chrome/browser/memo...
ios/chrome/browser/memory/memory_wedge_unittest.cc:12: namespace {
On 2015/04/28 18:43:40, Alexei Svitkine wrote:
> Use namespace memory_wedge to match the cc file.

Done.

[lpromero, 2015-04-28 22:02:10]

The CQ bit was checked by lpromero@chromium.org to run a CQ dry run

[lpromero, 2015-04-28 22:02:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1107963002/#ps80001
(title: "Review feedback")

[lpromero, 2015-04-28 22:02:45]

The CQ bit was unchecked by lpromero@chromium.org

[stuartmorgan, 2015-04-28 22:52:10]

LGTM with nits, but please fix the CL description: this does not add a memory
wedge that is controlled by a Finch experiment and allocates memory on launch.
It adds the foundation necessary to build such a thing, but has no actual effect
by itself.

(It's not really clear to me why the flag/experiment part of this is being done
upstream given that the only usage will downstream.)

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
File ios/chrome/browser/chrome_switches.cc (right):

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
ios/chrome/browser/chrome_switches.cc:18: // Defines the value of the memory
wedge to insert at cold launch.
in MB

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
ios/chrome/browser/chrome_switches.cc:19: const char kIOSMemoryWedgeSize[] =
"ios-memory-wedge-size";
It seems like an "-mb" suffix would be helpful here.

[lpromero, 2015-04-29 08:51:49]

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
File ios/chrome/browser/chrome_switches.cc (right):

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
ios/chrome/browser/chrome_switches.cc:18: // Defines the value of the memory
wedge to insert at cold launch.
On 2015/04/28 22:52:10, stuartmorgan wrote:
> in MB

Done.

https://codereview.chromium.org/1107963002/diff/80001/ios/chrome/browser/chro...
ios/chrome/browser/chrome_switches.cc:19: const char kIOSMemoryWedgeSize[] =
"ios-memory-wedge-size";
On 2015/04/28 22:52:10, stuartmorgan wrote:
> It seems like an "-mb" suffix would be helpful here.

Done.

[lpromero, 2015-04-29 11:06:53]

The CQ bit was checked by lpromero@chromium.org

[lpromero, 2015-04-29 11:06:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
stuartmorgan@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1107963002/#ps40002
(title: "Review feedback.")

[commit-bot: I haz the power, 2015-04-29 11:07:05]

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

[commit-bot: I haz the power, 2015-04-29 11:16:08]

Committed patchset #6 (id:40002)

[commit-bot: I haz the power, 2015-04-29 11:16:56]

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