Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2418)

Unified Diff: base/android/library_loader/library_prefetcher.cc

Issue 1001343002: Prefetch the native library from native code by parsing /proc/pid/maps. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address comments. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..2c6b6e0fcce61ff05556ff638c4ef817ce1e010d
--- /dev/null
+++ b/base/android/library_loader/library_prefetcher.cc
@@ -0,0 +1,197 @@
+// 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/logging.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+
+namespace base {
+namespace android {
+
+namespace {
+
+// Android defines the background priority to this value since at least 2009
pasko 2015/03/27 13:29:28 If THREAD_PRIORITY_BACKGROUND can change, let's ju
Benoit L 2015/03/27 15:44:52 Well, this adds clutter, and as you said, is highl
+// (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 std::string kSuffixesToMatch[] = {"libchrome.so", "base.apk"};
pasko 2015/03/27 13:29:28 <quote> ... we only allow static variables to cont
Benoit L 2015/03/27 15:44:52 Done.
+
+bool IsReadableAndPrivate(const std::string& flags) {
+ return flags.size() == 4 && flags[0] == 'r' && flags[3] == 'p';
+}
+
+bool FilenameMatchesSuffix(const std::string& filename) {
+ for (int i = 0; i < kSuffixesToMatchCount; i++) {
+ if (EndsWith(filename, kSuffixesToMatch[i], true)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+// For each range, reads a byte per page to force it into the page cache.
pasko 2015/03/27 17:51:54 can you add a comment saying that heap allocations
+void Prefetch(const std::vector<std::pair<uint64_t, uint64_t>>& ranges) {
+ for (const auto& range : ranges) {
+ const uint64_t page_mask = kPageSize - 1;
+ CHECK(!(range.first & page_mask) && !(range.second & page_mask));
+ 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;
+ addr += kPageSize) {
+ // Volatile is required to prevent the compiler from eliminating this
+ // loop.
+ dummy ^= *((volatile unsigned char*)addr);
+ }
+ }
+}
+
+} // namespace
+
+bool NativeLibraryPrefetcher::ParseMapping(const std::string& line,
pasko 2015/03/27 13:29:28 // static (same for the 2 other functions)
Benoit L 2015/03/27 15:44:51 Done.
+ Mapping* mapping) {
+ // Example:
+ // b6e62000-b6e64000 r-xp 00000000 b3:15 1204 /system/lib/libstdc++.so
+ // start-end flags offset device inode [filename]
+ std::vector<std::string> pieces;
+ base::SplitStringAlongWhitespace(line, &pieces);
+ size_t pieces_count = pieces.size();
+ // Filename is optional, so at least 5 fields.
+ if (pieces_count < 5) {
+ return false;
+ }
+
+ std::vector<std::string> start_end;
+ base::SplitString(pieces[0], '-', &start_end);
+ if (start_end.size() != 2) {
+ return false;
+ }
+ bool ok = base::HexStringToUInt64(start_end[0], &mapping->start);
+ ok &= base::HexStringToUInt64(start_end[1], &mapping->end);
+ mapping->flags = pieces[1];
+ ok &= base::HexStringToUInt64(pieces[2], &mapping->offset);
+ mapping->device = pieces[3];
+ ok &= base::StringToInt64(pieces[4], &mapping->inode);
+ if (!ok) {
+ return false;
+ }
+ if (pieces_count == 5) {
+ mapping->filename = std::string();
+ } else {
+ // Truncates filenames containing spaces, but these are not the droids we're
+ // looking for.
+ mapping->filename = pieces[5];
+ }
+ return true;
+}
+
+bool NativeLibraryPrefetcher::MappingMatches(const Mapping& mapping) {
+ return FilenameMatchesSuffix(mapping.filename) &&
+ IsReadableAndPrivate(mapping.flags); // Code mappings are private.
+}
+
+bool NativeLibraryPrefetcher::FindRanges(
+ std::vector<std::pair<uint64_t, uint64_t>>* ranges) {
+ base::ScopedFILE file(base::OpenFile(base::FilePath("/proc/self/maps"), "r"));
pasko 2015/03/27 13:29:27 OpenFile is the implementation detail of base::Fil
Benoit L 2015/03/27 15:44:52 We need a FILE*, and not a File. And File::Platfor
pasko 2015/03/27 17:51:54 ah, you are right, fgets, and there is no line by
+ if (!file.get()) {
+ return false;
+ }
+ std::vector<Mapping> mappings;
+ const size_t kMaxLineLength = 4096;
+ char line_buffer[kMaxLineLength];
+ while (char* line_str = fgets(line_buffer, kMaxLineLength - 1, file.get())) {
+ std::string line(line_str);
+ Mapping mapping;
+ if (!ParseMapping(line, &mapping)) {
+ continue;
+ }
+ if (MappingMatches(mapping)) {
+ mappings.push_back(mapping);
+ }
+ }
+ const std::string& libchrome_suffix = kSuffixesToMatch[0];
pasko 2015/03/27 13:29:27 nit: it's probably nice to separate this part with
Benoit L 2015/03/27 15:44:52 Done.
+ bool has_libchrome_mapping = false;
+ for (const Mapping& mapping : mappings) {
+ if (EndsWith(mapping.filename, libchrome_suffix, true)) {
+ has_libchrome_mapping = true;
+ break;
+ }
+ }
+ for (const Mapping& mapping : mappings) {
+ if (has_libchrome_mapping
+ && !EndsWith(mapping.filename, libchrome_suffix, true)) {
+ continue;
+ }
+ ranges->push_back(std::make_pair(mapping.start, mapping.end));
+ }
+ return true;
+}
+
+// Forks and waits for 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
pasko 2015/03/27 11:03:37 Forking does not fully 'isolate' because the proce
+// returns an incorrect address, only the forked process will crash.
+// - Not inflating the memory used by the main process uselessly, which could
+// increase its likelihood to be killed.
+// 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.
+bool NativeLibraryPrefetcher::ForkAndPrefetchNativeLibrary() {
+ // Looking for ranges is done before the fork, to avoid syscalls and/or
+ // memory allocations in the forked process, that can be problematic.
pasko 2015/03/27 17:51:54 s/ that can be problematic// does not add any use
+ std::vector<std::pair<uint64_t, uint64_t>> ranges;
+ if (!FindRanges(&ranges)) {
+ return false;
+ }
+ pid_t pid = fork();
pasko 2015/03/27 11:03:37 what if the child process deadlocks? It might happ
Benoit L 2015/03/27 15:44:51 This process doesn't do any heap allocation, and d
+ if (pid == 0) {
+ setpriority(PRIO_PROCESS, 0, kBackgroundPriority);
+ Prefetch(ranges);
+ exit(0);
pasko 2015/03/27 11:03:37 this would invoke destructors which we don't do no
Benoit L 2015/03/27 15:44:51 _exit(2) doesn't call the atexit() handlers. Done.
+ } 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;
+ }
+ if (WIFSIGNALED(status)) {
+ switch (WTERMSIG(status)) {
+ case SIGABRT:
+ case SIGBUS:
+ case SIGFPE:
+ case SIGILL:
+ case SIGSEGV:
+ LOG(WARNING) << "The native library prefetching process crashed ("
+ << pid << ")";
pasko 2015/03/27 11:03:37 break;
Benoit L 2015/03/27 15:44:52 Done.
pasko 2015/03/27 17:51:54 I like how this got done :)
+ case SIGINT:
+ case SIGKILL:
+ case SIGTERM:
+ LOG(WARNING) << "The native library prefetching process was killed "
pasko 2015/03/27 11:03:37 is there a reason to distinguish these cases? If y
Benoit L 2015/03/27 15:44:52 Done.
+ << "(" << pid << ")";
+ default:
+ break;
+ }
+ }
+ }
+ return false;
+ }
+}
+
+} // namespace android
+} // namespace base

Powered by Google App Engine
This is Rietveld 408576698