Transform ImagePreReader into PreReadFile.

chrome/app/file_pre_reader_win.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/app/file_pre_reader_win.h"
+
+#include <windows.h>
+#include <algorithm>
+
+#include "base/files/memory_mapped_file.h"
+#include "base/logging.h"
+#include "base/threading/thread_restrictions.h"
+#include "base/win/scoped_handle.h"
+#include "base/win/windows_version.h"
+
+namespace {
+
+// A helper function to touch all pages in the range
+// [base_addr, base_addr + length).
+void TouchPagesInRange(const void* base_addr, size_t length) {
+  DCHECK(base_addr != NULL);
+  DCHECK(length > 0);
+
+  // Get the system info so we know the page size. Also, make sure we use a
+  // non-zero value for the page size; GetSystemInfo() is hookable/patchable,
+  // and you never know what shenanigans someone could get up to.
+  SYSTEM_INFO system_info = {};
+  GetSystemInfo(&system_info);
+  if (system_info.dwPageSize == 0)
+    system_info.dwPageSize = 4096;
+
+  // We don't want to read outside the byte range (which could trigger an
+  // access violation), so let's figure out the exact locations of the first
+  // and final bytes we want to read.
+  volatile uint8 const* touch_ptr = reinterpret_cast<uint8 const*>(base_addr);

[grt (UTC plus 2), 2015/10/29 19:15:06]

uint8_t here and elsewhere

[fdoray, 2015/10/30 21:08:54]

Done.

+  volatile uint8 const* final_touch_ptr = touch_ptr + length - 1;
+
+  // Read the memory in the range [touch_ptr, final_touch_ptr] with a stride
+  // of the system page size, to ensure that it's been paged in.
+  uint8 dummy;
+  while (touch_ptr < final_touch_ptr) {
+    dummy = *touch_ptr;
+    touch_ptr += system_info.dwPageSize;
+  }
+  dummy = *final_touch_ptr;
+}
+
+}  // namespace
+
+bool FilePreReader::PreReadImage(const wchar_t* file_path, size_t step_size) {
+  base::ThreadRestrictions::AssertIOAllowed();
+  if (base::win::GetVersion() > base::win::VERSION_XP) {
+    // Vista+ branch. On these OSes, the forced reads through the DLL actually
+    // slows warm starts. The solution is to sequentially read file contents
+    // with an optional cap on total amount to read.
+    base::win::ScopedHandle file(

[grt (UTC plus 2), 2015/10/29 19:15:07]

please add a comment that base::File isn't used because of
FILE_FLAG_SEQUENTIAL_SCAN. better yet, add support for FILE_FLAG_SEQUENTIAL_SCAN
and use:
base::File file(file_path, FLAG_OPEN | FLAG_READ | FLAG_SHARE_DELETE |
FLAG_SEQUENTIAL_SCAN);

[fdoray, 2015/10/30 21:08:54]

Done. base::File::FLAG_SEQUENTIAL_SCAN was added in this CL
https://codereview.chromium.org/1424943006/

+        CreateFile(file_path, GENERIC_READ,
+                   FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
+                   OPEN_EXISTING, FILE_FLAG_SEQUENTIAL_SCAN, NULL));
+
+    if (!file.IsValid())
+      return false;
+
+    // Default to 1MB sequential reads.
+    const DWORD actual_step_size = std::max(static_cast<DWORD>(step_size),
+                                            static_cast<DWORD>(1024 * 1024));
+    LPVOID buffer =
+        ::VirtualAlloc(NULL, actual_step_size, MEM_COMMIT, PAGE_READWRITE);
+
+    if (buffer == NULL)
+      return false;
+
+    DWORD len;
+    size_t total_read = 0;

[grt (UTC plus 2), 2015/10/29 19:15:06]

file sizes are 64bits on windows, so uint64_t.
better yet, this variable is never used. get rid of it?

[fdoray, 2015/10/30 21:08:54]

Done.

+    while (::ReadFile(file.Get(), buffer, actual_step_size, &len, NULL) &&
+           len > 0) {
+      total_read += static_cast<size_t>(len);
+    }
+    ::VirtualFree(buffer, 0, MEM_RELEASE);
+  } else {
+    // WinXP branch. Here, reading the DLL from disk doesn't do
+    // what we want so instead we pull the pages into memory and touch pages at
+    // a stride. We use the system's page size as the stride, ignoring the
+    // passed in step_size, to make sure each page in the range is touched.
+    base::MemoryMappedFile file_memory_map;
+    CHECK(file_memory_map.Initialize(base::FilePath(file_path)));

[grt (UTC plus 2), 2015/10/29 19:15:06]

why crash here but not on line 62?

[fdoray, 2015/10/30 21:08:54]

Done. We shouldn't crash.

+    TouchPagesInRange(file_memory_map.data(), file_memory_map.length());
+  }
+
+  return true;
+}