Prefetch the native library from native code by parsing /proc/pid/maps.

base/android/library_loader/library_prefetcher.cc

+// Copyright 2015 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 "base/android/library_loader/library_prefetcher.h"
+
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/strings/string_util.h"
+
+namespace base {
+namespace android {
+
+namespace {
+
+// Android defines the background priority to this value since at least 2009
+// (see Process.java).
+const int kBackgroundPriority = 10;
+// Valid for all the Android architectures.
+const size_t kPageSize = 4096;
+// We may load directly from the APK.

[rmcilroy, 2015/04/07 12:17:27]

This comment is not very clear, could you make it more clear that you are
mentioning this due to the 'base.apk' being a suffix as well as libchrome.so

[Benoit L, 2015/04/16 13:28:48]

Done.

+const char* kSuffixesToMatch[] = {"libchrome.so", "base.apk"};
+
+bool IsReadableAndPrivate(const base::debug::MappedMemoryRegion& region) {
+  return region.permissions & base::debug::MappedMemoryRegion::READ &&
+         region.permissions & base::debug::MappedMemoryRegion::PRIVATE;
+}
+
+bool PathMatchesSuffix(const std::string& path) {
+  for (size_t i = 0; i < arraysize(kSuffixesToMatch); i++) {
+    if (EndsWith(path, kSuffixesToMatch[i], true)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+// For each range, reads a byte per page to force it into the page cache.
+// Heap allocations, syscalls and library functions are not allowed in this
+// function.
+// Returns true for success.
+bool Prefetch(const std::vector<std::pair<uintptr_t, uintptr_t>>& ranges) {
+  for (const auto& range : ranges) {
+    const uintptr_t page_mask = kPageSize - 1;
+    // If start or end is not page-aligned, parsing went wrong. It is better to
+    // exit with an error.
+    if ((range.first & page_mask) || (range.second & page_mask)) {
+      return false;  // CHECK() is not allowed here.
+    }
+    unsigned char* start_ptr = reinterpret_cast<unsigned char*>(range.first);
+    unsigned char* end_ptr = reinterpret_cast<unsigned char*>(range.second);
+    unsigned char dummy;
+    for (unsigned char* ptr = start_ptr; ptr < end_ptr; ptr += kPageSize) {
+      // Volatile is required to prevent the compiler from eliminating this
+      // loop.
+      dummy ^= *static_cast<volatile unsigned char*>(ptr);
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+// static
+bool NativeLibraryPrefetcher::IsGoodToPrefetch(
+    const base::debug::MappedMemoryRegion& region) {
+  return PathMatchesSuffix(region.path) &&
+         IsReadableAndPrivate(region);  // .text and .data mappings are private.
+}
+
+// static
+void NativeLibraryPrefetcher::TakeOnlyLibchromeRangesIfPossible(

[rmcilroy, 2015/04/07 12:17:27]

nit - /s/Libchrome/LibChrome

[Benoit L, 2015/04/16 13:28:47]

Acknowledged.

+    const std::vector<base::debug::MappedMemoryRegion>& regions,
+    std::vector<AddressRange>* ranges) {
+  const std::string& libchrome_suffix = kSuffixesToMatch[0];

[rmcilroy, 2015/04/07 12:17:27]

It would be better to have "libchrome.so" as a separate constant  so that this
code doesn't have to depend on the order of the kSuffixesToMatch array.

[Benoit L, 2015/04/16 13:28:48]

Done.

+  bool has_libchrome_region = false;
+  for (const base::debug::MappedMemoryRegion& region : regions) {
+    if (EndsWith(region.path, libchrome_suffix, true)) {
+      has_libchrome_region = true;
+      break;
+    }
+  }
+  for (const base::debug::MappedMemoryRegion& region : regions) {
+    if (has_libchrome_region &&
+        !EndsWith(region.path, libchrome_suffix, true)) {
+      continue;
+    }
+    ranges->push_back(std::make_pair(region.start, region.end));
+  }
+}
+
+// static
+bool NativeLibraryPrefetcher::FindRanges(std::vector<AddressRange>* ranges) {
+  std::string proc_maps;
+  if (!base::debug::ReadProcMaps(&proc_maps))
+    return false;
+  std::vector<base::debug::MappedMemoryRegion> regions;
+  if (!base::debug::ParseProcMaps(proc_maps, &regions))
+    return false;
+
+  std::vector<base::debug::MappedMemoryRegion> regions_to_prefetch;
+  for (const auto& region : regions) {
+    if (IsGoodToPrefetch(region)) {
+      regions_to_prefetch.push_back(region);
+    }
+  }
+
+  TakeOnlyLibchromeRangesIfPossible(regions_to_prefetch, ranges);

[rmcilroy, 2015/04/07 12:17:27]

nit - FilterLibChromeRangesOnlyIfPossible

[Benoit L, 2015/04/16 13:28:48]

Done.

+  return true;
+}
+
+// static
+bool NativeLibraryPrefetcher::ForkAndPrefetchNativeLibrary() {
+  // Looking for ranges is done before the fork, to avoid syscalls and/or memory
+  // allocations in the forked process. The child process inherits the lock
+  // state of its parent thread. It cannot rely on being able to acquire any
+  // lock (unless special care is taken in a pre-fork handler), including being
+  // able to call malloc().

[rmcilroy, 2015/04/07 12:17:27]

As you state here, forking a process with threads is full of gotchas and could
potentially cause deadlocks. Are you really sure it is worth forking here? I'm
not sure the reasons listed in library_prefetcher.h are very strong - I would
actually prefer that the main process crashed if there were errors in the
parsing code, at least that way we would know about it and could fix it (also
the crazy linker already does a bunch of /proc/self/maps processing
unconditionally and hasn't had problems doing so so far). I'm also not convinced
that inflating memory usage is an issue since these pages will be clean and so
the kernel would be more likely to simply unmap them rather than killing the
Chrome process under memory pressure.

If you are sure you need to do this in a separate process, did you consider
doing a fork and execvp instead (perhaps transferring the /proc/<pid>/maps file
as a file descriptor to the child process and doing the parsing on the child
process)?  This would be a bit more work but would be much safer IMO.

[Benoit L, 2015/04/16 13:28:47]

Forking a new process was brought up as a nice solution after discussions with
simonb@ and primiano@, partially traced in this (abandonned) CL comments :
https://codereview.chromium.org/958473003/ 

I still believe that this is important. Also in other discussions, inflating
chrome's PSS was considered as an important point, as well as having the page
not be referenced, instead of clean but referenced.
Well, this would also require to a separate "one-trick pony" binary. I think
that it is more trouble than necessary.

[rmcilroy, 2015/04/23 15:09:42]

Makes sense. Thanks for the explination.
 
Yeah you are probably right. I was kindof thinking about the Linux world where
we could launch the chrome executable with a different process-type argument,
but this is a bit more tricky in the Android world.  OK, I guess this is fine
with the warnings.

+  std::vector<AddressRange> ranges;
+  if (!FindRanges(&ranges))
+    return false;
+  pid_t pid = fork();
+  if (pid == 0) {
+    setpriority(PRIO_PROCESS, 0, kBackgroundPriority);
+    // _exit() doesn't call the atexit() handlers.
+    _exit(Prefetch(ranges) ? 0 : 1);
+  } else {
+    if (pid < 0) {
+      return false;
+    }
+    int status;
+    const pid_t result = HANDLE_EINTR(waitpid(pid, &status, 0));
+    if (result == pid) {
+      if (WIFEXITED(status)) {
+        return WEXITSTATUS(status) == 0;
+      }
+    }
+    return false;
+  }
+}
+
+}  // namespace android
+}  // namespace base