Don't realize existing parts of an extended memory-mapped file.

Don't realize existing parts of an extended memory-mapped file.

Realizing the entire file when extending a file is expensive and
unnecessary if we assume that the existing file is not sparse.  Just
realize the extension.

This is important for persistent metrics which creates a "spare" file
for use by the next run of the browser.  To realize it again would
negate most of the benefit of creating it in the first place.

BUG=717331
Review-Url: https://codereview.chromium.org/2939073002
Cr-Commit-Position: refs/heads/master@{#479718}
Committed: https://chromium.googlesource.com/chromium/src/+/f838936df9cdc2d95d64fb7cbae76a6ecd927cdd

[bcwhite, 2017-06-15 14:20:50]

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

[commit-bot: I haz the power, 2017-06-15 14:20:56]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bcwhite, 2017-06-15 15:13:29]

bcwhite@chromium.org changed reviewers:
+ mark@chromium.org

[Mark Mentovai, 2017-06-15 15:31:24]

Might we pick up an old sparse file that’s sparse because we crashed or failed
to delete it while attempting to make it not sparse during a previous run?

https://codereview.chromium.org/2939073002/diff/1/base/files/memory_mapped_fi...
File base/files/memory_mapped_file_posix.cc (right):

https://codereview.chromium.org/2939073002/diff/1/base/files/memory_mapped_fi...
base/files/memory_mapped_file_posix.cc:148: return false;  // Can't write? Not
viable.
If we hit this, does something cause the file to be deleted?

[bcwhite, 2017-06-15 15:38:19]

> Might we pick up an old sparse file that’s sparse because we crashed or failed
> to delete it while attempting to make it not sparse during a previous run?

It's possible, yes. If the file was created and written to in a sparse fashion
using standard I/O calls or, as you say, the process crashed during the previous
attempt to create it.

Both of these seem unlikely conditions and both could still cause problems if
accessed in the READ_WRITE (no EXTEND) mode.

Creation of the spare file uses a temp name and only assigns it the final name
upon successful mapping/extension.

https://codereview.chromium.org/2939073002/diff/1/base/files/memory_mapped_fi...
File base/files/memory_mapped_file_posix.cc (right):

https://codereview.chromium.org/2939073002/diff/1/base/files/memory_mapped_fi...
base/files/memory_mapped_file_posix.cc:148: return false;  // Can't write? Not
viable.
On 2017/06/15 15:31:23, Mark Mentovai wrote:
> If we hit this, does something cause the file to be deleted?

With regard to persistent histograms, the unused file will get rotated out and a
new one (the "spare" one) used during the next run.  The one rotated out will
get deleted.

Creation of the spare file uses a temp file and only assigns it the final name
upon successful mapping/extension.

[Mark Mentovai, 2017-06-15 15:40:49]

Thanks. LGTM in that case.

[commit-bot: I haz the power, 2017-06-15 15:46:00]

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

[commit-bot: I haz the power, 2017-06-15 15:46:01]

Dry run: This issue passed the CQ dry run.

[bcwhite, 2017-06-15 15:46:06]

Description was changed from

==========
Don't realize existing parts of memory-mapped file.

Realizing the entire file when extending a file is expensive and
unnecessary if we assume that the existing file is not sparse.  Just
realize the extension.

This is important for persistent metrics which creates a "spare" file
for use by the next run of the browser.  To try to realize it would
negate most of the benefit of creating it.

BUG=717331
==========

to

==========
Don't realize existing parts of an extended memory-mapped file.

Realizing the entire file when extending a file is expensive and
unnecessary if we assume that the existing file is not sparse.  Just
realize the extension.

This is important for persistent metrics which creates a "spare" file
for use by the next run of the browser.  To realize it again would
negate most of the benefit of creating it in the first place.

BUG=717331
==========

[bcwhite, 2017-06-15 15:46:13]

The CQ bit was checked by bcwhite@chromium.org

[commit-bot: I haz the power, 2017-06-15 15:46:25]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-15 15:51:37]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1497541573439030, "parent_rev":
"84f44b2a70b37f07c2a3e23c7c79208342aca142", "commit_rev":
"f838936df9cdc2d95d64fb7cbae76a6ecd927cdd"}

[commit-bot: I haz the power, 2017-06-15 15:51:49]

Description was changed from

==========
Don't realize existing parts of an extended memory-mapped file.

Realizing the entire file when extending a file is expensive and
unnecessary if we assume that the existing file is not sparse.  Just
realize the extension.

This is important for persistent metrics which creates a "spare" file
for use by the next run of the browser.  To realize it again would
negate most of the benefit of creating it in the first place.

BUG=717331
==========

to

==========
Don't realize existing parts of an extended memory-mapped file.

Realizing the entire file when extending a file is expensive and
unnecessary if we assume that the existing file is not sparse.  Just
realize the extension.

This is important for persistent metrics which creates a "spare" file
for use by the next run of the browser.  To realize it again would
negate most of the benefit of creating it in the first place.

BUG=717331

Review-Url: https://codereview.chromium.org/2939073002
Cr-Commit-Position: refs/heads/master@{#479718}
Committed:
https://chromium.googlesource.com/chromium/src/+/f838936df9cdc2d95d64fb7cbae7...
==========

[commit-bot: I haz the power, 2017-06-15 15:51:50]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/f838936df9cdc2d95d64fb7cbae7...