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

base/android/library_loader/library_prefetcher.cc

Index: base/android/library_loader/library_prefetcher.cc
diff --git a/base/android/library_loader/library_prefetcher.cc b/base/android/library_loader/library_prefetcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..84038a6bbafa58f387a374bf7aef10ae7e6c24b7
--- /dev/null
+++ b/base/android/library_loader/library_prefetcher.cc
@@ -0,0 +1,167 @@
+// 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 <stdlib.h>
+#include <sys/resource.h>
+#include <unistd.h>
+#include <utility>
+#include <vector>
+
+namespace base {
+namespace android {
+
+FileLineIterator::FileLineIterator(const base::FilePath& path) {
+  file_ = base::OpenFile(path, "r");
+}
+
+FileLineIterator::~FileLineIterator() {
+  if (file_) {
+    base::CloseFile(file_);
+  }
+}
+
+std::string FileLineIterator::NextLine() {
+  if (!file_) {
+    return std::string();
+  }
+  char* line = fgets(line_buffer_, kMaxLineLength - 1, file_);

[pasko, 2015/03/24 17:10:36]

why so low level and complex with FILE* and fgets etc? You can use
ReadFileToString() and StringTokenizer. Then it is possible to use StringPiece.

Amount of allocations would probably be the same in practice.

[Benoit L, 2015/03/25 10:36:18]

I would prefer to avoid an unbounded memory allocation in the parent process, if
possible.

+  return line ? std::string(line) : std::string();
+}
+
+ProcMapsIterator::ProcMapsIterator(FileLineIterator* line_iterator)
+    : line_iterator_(line_iterator) {
+}
+
+bool ProcMapsIterator::Next(uint64_t* start,
+                            uint64_t* end,
+                            std::string* flags,
+                            uint64_t* offset,
+                            std::string* device,
+                            int64_t* inode,
+                            std::string* filename) {

[rmcilroy, 2015/03/24 15:18:26]

nit - this might be neater wrapping up the arguments as a struct.

[Benoit L, 2015/03/25 10:36:19]

Done.

+  std::string line = line_iterator_->NextLine();
+  if (line.size() == 0) {
+    return false;
+  }
+  // Example:
+  // b6e62000-b6e64000 r-xp 00000000 b3:15 1204 /system/lib/libstdc++.so
+  // start-end flags offset device inode [filename]
+  std::string line_str(line);
+  std::vector<std::string> pieces;
+  base::SplitStringAlongWhitespace(line_str, &pieces);
+  size_t pieces_count = pieces.size();
+  // Filename is optional
+  DCHECK((pieces_count == 5) || (pieces_count == 6));

[rmcilroy, 2015/03/24 15:18:26]

Either do this as a CHECK or add an if to only to the code below if this is true
(preferred I think), otherwise the code below could do an buffer-overflow on
release builds if the format changes in the future.

[Benoit L, 2015/03/25 10:36:18]

Done.

+
+  const char* start_end = pieces[0].c_str();
+  char* delimiter_ptr, *dummy;
+  static_assert(sizeof(long long) == sizeof(int64_t),
+                "long long is not int64_t");  // This way we can use strtoll().

[pasko, 2015/03/24 17:10:36]

StringToInt64?

[Benoit L, 2015/03/25 10:36:19]

Done.

+  *start = strtoull(start_end, &delimiter_ptr, 16);
+  *end = strtoull(delimiter_ptr + 1, &dummy, 16);

[rmcilroy, 2015/03/24 15:18:26]

nit - I would prefer you just used base::SplitString here to split the start and
end with the dash.

[Benoit L, 2015/03/25 10:36:18]

Done.

+  *flags = pieces[1];
+  *offset = strtoull(pieces[2].c_str(), &dummy, 16);
+  *device = pieces[3];
+  *inode = strtoll(pieces[4].c_str(), &dummy, 10);
+  if (pieces_count == 5) {
+    *filename = std::string();
+  } else {
+    *filename = pieces[5];
+  }
+  return true;
+}
+
+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.
+const int kSuffixesToMatchCount = 2;
+const char* kSuffixesToMatch[] = {"libchrome.so", "base.apk"};

[pasko, 2015/03/24 17:10:36]

why base.apk? is the chrome apk always renamed to base.apk? Why not just .apk?

[Benoit L, 2015/03/25 10:36:19]

According to the android source code (see
PackageInstallerSession#validateInstallLocked), yes, unless we are using split
apks.

+
+bool IsReadableAndPrivate(const char* flags) {
+  return flags && flags[0] == 'r' && flags[3] == 'p';

[rmcilroy, 2015/03/24 15:18:26]

nit - add a check here that flags is long enough to avoid buffer overflows.

[Benoit L, 2015/03/25 10:36:18]

Done.

+}
+
+bool FilenameMatchesSuffix(const char* filename) {
+  if (!filename)
+    return false;
+  const size_t filename_length = strlen(filename);
+  for (int i = 0; i < kSuffixesToMatchCount; i++) {
+    const size_t suffix_length = strlen(kSuffixesToMatch[i]);
+    if (filename_length < suffix_length)
+      continue;
+    if (!strcmp(kSuffixesToMatch[i],
+                filename + filename_length - suffix_length))
+      return true;
+  }
+  return false;
+}
+
+// For each range, reads a byte per page to force it into the page cache.
+void Prefetch(const std::vector<std::pair<uint64_t, uint64_t>>& ranges) {
+  for (const auto& range : ranges) {
+    unsigned char* start_address = (unsigned char*) range.first;
+    unsigned char* end_address = (unsigned char*) range.second;
+    unsigned char dummy;
+    for (unsigned char* addr = start_address; addr < end_address - sizeof(int);

[pasko, 2015/03/24 17:10:36]

s/ - sizeof(int)// ?

you can CHECK(!(start_address & 0xFFF) && !(end_address & 0xFFF)) to make it
even more straightforward.

[Benoit L, 2015/03/25 10:36:18]

Done.

+         addr += kPageSize) {
+      // Volatile is required to prevent the compiler from eliminating this
+      // loop.
+      dummy ^= *((volatile unsigned char*) addr);
+    }
+  }
+}
+
+}  // namespace
+
+std::vector<std::pair<uint64_t, uint64_t>> FindRanges(
+    FileLineIterator* file_line_iterator) {
+  std::vector<std::pair<uint64_t, uint64_t>> ranges;
+  ProcMapsIterator maps_iterator(file_line_iterator);
+  uint64_t start, end, offset;
+  int64_t inode;
+  std::string flags, device, filename;
+
+  while (maps_iterator.Next(&start, &end, &flags, &offset, &device, &inode,
+                            &filename)) {
+    if (FilenameMatchesSuffix(filename.c_str()) &&
+        IsReadableAndPrivate(flags.c_str())) {
+      ranges.push_back(std::make_pair(start, end));

[pasko, 2015/03/24 17:10:36]

We could potentially have both libchrome.so and .apk mapped, in this case we
need to prefetch only libchrome.so (like on those Samsung devices where we
cannot mmap from apk, it does not mean smb else could not mmap the apk in our
process before us, right?)

Having multiple ranges scares me, we are not deterministic at walking through
the ranges, i.e. injecting a source of variance that'd be impossible to find.
Can we just on encountering more than one range just record a histogram and walk
away?

[Benoit L, 2015/03/25 10:36:18]

Well, we have several ranges in pretty much all cases right now:
- .text can be split, with a hole in the middle (related to shared relro IIRC)
- .data is separate from .text (not the same permissions, obviously)

So having several ranges is pretty much unavoidable. However code mappings are
private, which is why we are checking this above.

[pasko, 2015/03/25 12:40:53]

Got it. As per offline discussion, on later systems regions both libchrome.so
and base.apk can be mapped (both private and shared), and we may introduce extra
IO if prefetching the wrong one (it would not fail, just a slowdown and a source
of non-determinism).

So to deal with that let's:
if there exists at least one range from libchrome.so:
  prefetch only libchrome.so ranges
else:
  prefetch base.apk ranges

[rmcilroy, 2015/03/25 15:12:18]

It's due to packed relocations, but yes we will need to support it.

+    }
+  }
+  return ranges;
+}
+
+// Forks a process prefetching the native library. This is done in a forked
+// process for the following reasons:
+// - Isolating the main process from mistakes in the parsing. If the parsing
+//   returns an incorrect address, only the forked process will crash.

[rmcilroy, 2015/03/24 15:18:26]

The parsing is done on the main process though.

[Benoit L, 2015/03/25 10:36:18]

Acknowledged.

+// - Not inflating the memory used by the main process uselessly, which could
+//   increase its likelihood to be killed.
+// - Have the pages clean and not referenced, to make them easy for the kernel
+//   to evict, minimizing disruption caused by this prefetching.

[rmcilroy, 2015/03/24 15:18:26]

Does forking actually helps this - the pages should be clean even if they were
Prefetched by the main process (unless the main process modifies them but in
that case they wouldn't be clean and unreferenced even if they are prefetched by
the forked process). 

[Benoit L, 2015/03/25 10:36:18]

You are right, removed this comment.
Done.

+// The forked process has background priority and, since it is not declared to
+// the android runtime, can be killed at any time, which is not an issue here.
+void ForkAndPrefetchNativeLibrary() {
+  // Looking for ranges is done before the fork, to avoid syscalls and/or
+  // memory allocations in the forked process, that can be problematic.
+  FileLineIterator file_line_iterator(base::FilePath("/proc/self/maps"));
+  std::vector<std::pair<uint64_t, uint64_t>> ranges =
+      FindRanges(&file_line_iterator);
+  pid_t pid = fork();
+  if (pid == 0) {
+    setpriority(PRIO_PROCESS, 0, kBackgroundPriority);
+    Prefetch(ranges);
+    exit(0);
+  }
+}
+
+}  // namespace android
+}  // namespace base