Try to speed up SimpleCache index re-sync on Windows.

Try to speed up SimpleCache index re-sync on Windows.

FindFirstFile[Ex]/FindNextFile backing FileEnumerator there
already fill in all the information we need, so no need to get
it again via GetFileAttributesEx (backing base::GetFileInfo)

On POSIX, of course, readdir_r doesn't provide file + timestamp info,
so the separate stat is still required.

This helps a lot on my not-quite-micro benchmarks (470ms -> 230ms or so),
but real-life long-tail measurements can be orders of magnitude slower,
so might not have the same root cause. Still, seems worthwhile since
this can be a startup background task reasonably frequently.

(Side note: Windows does seem to have atime, though it's documented as
 basically a*date* on FAT. It may be worth trying to use it to see if it
 helps the hit rate a bit in cases where accurate index info isn't available;
 might be too rare to evaluate, though)

BUG=705543
Review-Url: https://codereview.chromium.org/2774923002
Cr-Commit-Position: refs/heads/master@{#459823}
Committed: https://chromium.googlesource.com/chromium/src/+/5f5b9df55357e7d065bc4ecbfda8793840c18880

[Maks Orlovich, 2017-03-24 14:51:18]

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

[commit-bot: I haz the power, 2017-03-24 14:51:51]

Dry run: 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-03-24 16:28:22]

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

[commit-bot: I haz the power, 2017-03-24 16:28:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Maks Orlovich, 2017-03-24 17:39:15]

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

[commit-bot: I haz the power, 2017-03-24 17:39:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Maks Orlovich, 2017-03-24 17:46:54]

Seems like my test machine has an SSD, which may explain why its numbers are
significantly better than typical (and at least comparable to other SSDs), but
it's quite possible that this won't help much in the cold case. Sadly I don't
know enough about Windows guts to answer (perhaps there is someone I can ask?)

I figure we can use SimpleCache.Http.IndexEntriesRestored
and SimpleCache.Http.IndexRestoreTime to monitor effects of this over time.

[commit-bot: I haz the power, 2017-03-24 18:21:26]

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

[commit-bot: I haz the power, 2017-03-24 18:21:27]

Dry run: This issue passed the CQ dry run.

[Maks Orlovich, 2017-03-27 13:49:01]

morlovich@chromium.org changed reviewers:
+ jkarlin@chromium.org

[Maks Orlovich, 2017-03-27 13:49:02]

Not sure whom to ask, so please feel encouraged to reassign as appropriate (and
actually I thought I mailed this out a few days back, oops).

[jkarlin, 2017-03-27 14:09:16]

Looks good, and the wins look promising. 

Be sure to create a bug on and add a BUG= label to the description.

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_index_file.cc (right):

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_index_file.cc:124: base::Time last_accessed,
Could we instead take a 'const base::FileInfo& file_info' here?

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_index_file.h (right):

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_index_file.h:108: typedef base::Callback<void(const
base::FilePath&,
While we're here, change this to using EntryFileCallback = ...;

[Maks Orlovich, 2017-03-27 15:22:32]

Description was changed from

==========
Try to speed up SimpleCache index re-sync on Windows.

FindFirstFile[Ex]/FindNextFile backing FileEnumerator there
already fill in all the information we need, so no need to get
it again via GetFileAttributesEx (backing base::GetFileInfo)

On POSIX, of course, readdir_r doesn't provide file + timestamp info,
so the separate stat is still required.

This helps a lot on my not-quite-micro benchmarks (470ms -> 230ms or so),
but real-life long-tail measurements can be orders of magnitude slower,
so might not have the same root cause. Still, seems worthwhile since
this can be a startup background task reasonably frequently.

(Side note: Windows does seem to have atime, though it's documented as
 basically a*date* on FAT. It may be worth trying to use it to see if it
 helps the hit rate a bit in cases where accurate index info isn't available;
 might be too rare to evaluate, though)

BUG=
==========

to

==========
Try to speed up SimpleCache index re-sync on Windows.

FindFirstFile[Ex]/FindNextFile backing FileEnumerator there
already fill in all the information we need, so no need to get
it again via GetFileAttributesEx (backing base::GetFileInfo)

On POSIX, of course, readdir_r doesn't provide file + timestamp info,
so the separate stat is still required.

This helps a lot on my not-quite-micro benchmarks (470ms -> 230ms or so),
but real-life long-tail measurements can be orders of magnitude slower,
so might not have the same root cause. Still, seems worthwhile since
this can be a startup background task reasonably frequently.

(Side note: Windows does seem to have atime, though it's documented as
 basically a*date* on FAT. It may be worth trying to use it to see if it
 helps the hit rate a bit in cases where accurate index info isn't available;
 might be too rare to evaluate, though)

BUG=705543
==========

[Maks Orlovich, 2017-03-27 15:25:44]

(Bug of sorts added)

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_index_file.cc (right):

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_index_file.cc:124: base::Time last_accessed,
On 2017/03/27 14:09:15, jkarlin wrote:
> Could we instead take a 'const base::FileInfo& file_info' here?

We don't have a base::FileInfo on the Windows path --- we have a
base::FileEnumerator::FileInfo, which is a different type.

(Not using FileEnumerator on POSIX appears to be done on purpose:

https://chromium.googlesource.com/chromium/src/+/139e21b4cc0ca376b3fdcd13821e...
)

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_index_file.h (right):

https://codereview.chromium.org/2774923002/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_index_file.h:108: typedef base::Callback<void(const
base::FilePath&,
On 2017/03/27 14:09:15, jkarlin wrote:
> While we're here, change this to using EntryFileCallback = ...;

Done.

[jkarlin, 2017-03-27 15:30:24]

jkarlin@chromium.org changed reviewers:
+ asanka@chromium.org

[jkarlin, 2017-03-27 15:30:25]

lgtm, though I'm not a net/ owner. Adding asanka@.

[asanka, 2017-03-27 16:10:43]

lgtm

[Maks Orlovich, 2017-03-27 16:23:54]

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

[commit-bot: I haz the power, 2017-03-27 16:24:17]

Dry run: 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-03-27 17:29:49]

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

[commit-bot: I haz the power, 2017-03-27 17:29:50]

Dry run: This issue passed the CQ dry run.

[Maks Orlovich, 2017-03-27 17:36:13]

The CQ bit was checked by morlovich@chromium.org

[commit-bot: I haz the power, 2017-03-27 17:37:12]

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-03-27 17:55:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490636173665550,
"parent_rev": "d6eeaf9cc7296400931c598ca86beb4c82cfa103", "commit_rev":
"5f5b9df55357e7d065bc4ecbfda8793840c18880"}

[commit-bot: I haz the power, 2017-03-27 17:55:50]

Description was changed from

==========
Try to speed up SimpleCache index re-sync on Windows.

FindFirstFile[Ex]/FindNextFile backing FileEnumerator there
already fill in all the information we need, so no need to get
it again via GetFileAttributesEx (backing base::GetFileInfo)

On POSIX, of course, readdir_r doesn't provide file + timestamp info,
so the separate stat is still required.

This helps a lot on my not-quite-micro benchmarks (470ms -> 230ms or so),
but real-life long-tail measurements can be orders of magnitude slower,
so might not have the same root cause. Still, seems worthwhile since
this can be a startup background task reasonably frequently.

(Side note: Windows does seem to have atime, though it's documented as
 basically a*date* on FAT. It may be worth trying to use it to see if it
 helps the hit rate a bit in cases where accurate index info isn't available;
 might be too rare to evaluate, though)

BUG=705543
==========

to

==========
Try to speed up SimpleCache index re-sync on Windows.

FindFirstFile[Ex]/FindNextFile backing FileEnumerator there
already fill in all the information we need, so no need to get
it again via GetFileAttributesEx (backing base::GetFileInfo)

On POSIX, of course, readdir_r doesn't provide file + timestamp info,
so the separate stat is still required.

This helps a lot on my not-quite-micro benchmarks (470ms -> 230ms or so),
but real-life long-tail measurements can be orders of magnitude slower,
so might not have the same root cause. Still, seems worthwhile since
this can be a startup background task reasonably frequently.

(Side note: Windows does seem to have atime, though it's documented as
 basically a*date* on FAT. It may be worth trying to use it to see if it
 helps the hit rate a bit in cases where accurate index info isn't available;
 might be too rare to evaluate, though)

BUG=705543

Review-Url: https://codereview.chromium.org/2774923002
Cr-Commit-Position: refs/heads/master@{#459823}
Committed:
https://chromium.googlesource.com/chromium/src/+/5f5b9df55357e7d065bc4ecbfda8...
==========

[commit-bot: I haz the power, 2017-03-27 17:55:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5f5b9df55357e7d065bc4ecbfda8...