Skip to content

Commit

Permalink
android: Optionally madvise(MADV_RANDOM) on .text.
Browse files Browse the repository at this point in the history
This CL adds:

- "anchor" functions at the beginning and end of .text: these are
  already used in lightweight_cygprofile.cc, and require the orderfile
  to be properly constructed, which is the case since
  https://chromium-review.googlesource.com/753882. These functions are
  trickier for regular builds, as they use --icf=all.
- madvise(MADV_RANDOM): disable kernel readahead on a given range. This makes
  the reporting logic for code residency clearer, as it actually shows which
  pages are requested. This has to be called as early as possible and from all
  processes, and may be enabled in all builds at a later date. This is
  controlled by a new command-line flag, madvise-random-executable-code.

Bug: 758566
Change-Id: I8c7a5ca759b355b9e632d6ad027f8d9cd8fb84e8
Reviewed-on: https://chromium-review.googlesource.com/776895
Commit-Queue: Benoit L <[email protected]>
Reviewed-by: Camille Lamy <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Reviewed-by: Egor Pasko <[email protected]>
Reviewed-by: agrieve <[email protected]>
Reviewed-by: Matthew Cary <[email protected]>
Cr-Commit-Position: refs/heads/master@{#518639}
  • Loading branch information
Benoit Lize authored and Commit Bot committed Nov 22, 2017
1 parent ff6d2f3 commit 411a34f
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 27 deletions.
2 changes: 2 additions & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ jumbo_component("base") {
"android/jni_utils.h",
"android/jni_weak_ref.cc",
"android/jni_weak_ref.h",
"android/library_loader/anchor_functions.cc",
"android/library_loader/anchor_functions.h",
"android/library_loader/library_load_from_apk_status_codes.h",
"android/library_loader/library_loader_hooks.cc",
"android/library_loader/library_loader_hooks.h",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public void asyncPrefetchLibrariesToMemory() {
if (coldStart && CommandLine.getInstance().hasSwitch("log-native-library-residency")) {
// nativePeriodicallyCollectResidency() sleeps, run it on another thread,
// and not on the AsyncTask thread pool.
new Thread(() -> nativePeriodicallyCollectResidency()).run();
new Thread(LibraryLoader::nativePeriodicallyCollectResidency).start();
return;
}

Expand Down
70 changes: 70 additions & 0 deletions base/android/library_loader/anchor_functions.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2017 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/anchor_functions.h"

#include "base/logging.h"

// These functions are here to, respectively:
// 1. Check that functions are ordered
// 2. Delimit the start of .text
// 3. Delimit the end of .text
//
// (2) and (3) require a suitably constructed orderfile, with these
// functions at the beginning and end. (1) doesn't need to be in it.
//
// These functions are weird: this is due to ICF (Identical Code Folding).
// The linker merges functions that have the same code, which would be the case
// if these functions were empty, or simple.
// Gold's flag --icf=safe will *not* alias functions when their address is used
// in code, but as of November 2017, we use the default setting that
// deduplicates function in this case as well.
//
// Thus these functions are made to be unique, using inline .word in assembly.
//
// Note that code |CheckOrderingSanity()| below will make sure that these
// functions are not aliased, in case the toolchain becomes really clever.
extern "C" {

void dummy_function_to_check_ordering() {
asm(".word 0xe19c683d");
asm(".word 0xb3d2b56");
}

void dummy_function_to_anchor_text() {
asm(".word 0xe1f8940b");
asm(".word 0xd5190cda");
}

void dummy_function_at_the_end_of_text() {
asm(".word 0x133b9613");
asm(".word 0xdcd8c46a");
}

} // extern "C"

namespace base {
namespace android {

const size_t kStartOfText =
reinterpret_cast<size_t>(dummy_function_to_anchor_text);
const size_t kEndOfText =
reinterpret_cast<size_t>(dummy_function_at_the_end_of_text);

void CheckOrderingSanity() {
// The linker usually keeps the input file ordering for symbols.
// dummy_function_to_anchor_text() should then be after
// dummy_function_to_check_ordering() without ordering.
// This check is thus intended to catch the lack of ordering.
CHECK_LT(kStartOfText,
reinterpret_cast<size_t>(&dummy_function_to_check_ordering));
CHECK_LT(kStartOfText, kEndOfText);
CHECK_LT(kStartOfText,
reinterpret_cast<size_t>(&dummy_function_to_check_ordering));
CHECK_LT(kStartOfText, reinterpret_cast<size_t>(&CheckOrderingSanity));
CHECK_GT(kEndOfText, reinterpret_cast<size_t>(&CheckOrderingSanity));
}

} // namespace android
} // namespace base
23 changes: 23 additions & 0 deletions base/android/library_loader/anchor_functions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2017 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.

#ifndef BASE_ANDROID_LIBRARY_LOADER_ANCHOR_FUNCTIONS_H_
#define BASE_ANDROID_LIBRARY_LOADER_ANCHOR_FUNCTIONS_H_

#include <cstdint>

namespace base {
namespace android {

// Start and end of .text, respectively.
extern const size_t kStartOfText;
extern const size_t kEndOfText;

// Basic CHECK()s ensuring that the symbols above are correctly set.
void CheckOrderingSanity();

} // namespace android
} // namespace base

#endif // BASE_ANDROID_LIBRARY_LOADER_ANCHOR_FUNCTIONS_H_
17 changes: 11 additions & 6 deletions base/android/library_loader/library_loader_hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include "base/android/library_loader/library_load_from_apk_status_codes.h"
#include "base/android/library_loader/library_prefetcher.h"
#include "base/at_exit.h"
#include "base/base_switches.h"
#include "base/command_line.h"
#include "base/metrics/histogram.h"
#include "base/metrics/histogram_macros.h"
#include "jni/LibraryLoader_jni.h"
Expand Down Expand Up @@ -167,13 +169,16 @@ void SetLibraryLoadedHook(LibraryLoadedHook* func) {
static jboolean JNI_LibraryLoader_LibraryLoaded(
JNIEnv* env,
const JavaParamRef<jobject>& jcaller) {
if (g_native_initialization_hook && !g_native_initialization_hook()) {
return false;
}
if (g_registration_callback == NULL) {
return true;
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kMadviseRandomExecutableCode)) {
NativeLibraryPrefetcher::MadviseRandomText();
}
return g_registration_callback(env, NULL);

if (g_native_initialization_hook && !g_native_initialization_hook())
return false;
if (g_registration_callback && !g_registration_callback(env, nullptr))
return false;
return true;
}

void LibraryLoaderExitHook() {
Expand Down
49 changes: 30 additions & 19 deletions base/android/library_loader/library_prefetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include <utility>
#include <vector>

#include "base/android/library_loader/anchor_functions.h"
#include "base/bits.h"
#include "base/files/file.h"
#include "base/format_macros.h"
#include "base/macros.h"
Expand Down Expand Up @@ -82,6 +84,19 @@ bool Prefetch(const std::vector<std::pair<uintptr_t, uintptr_t>>& ranges) {
return true;
}

// Returns the start and end of .text, aligned to the lower and upper page
// boundaries, respectively.
NativeLibraryPrefetcher::AddressRange GetTextRange() {
// |kStartOftext| may not be at the beginning of a page, since .plt can be
// before it, yet in the same mapping for instance.
size_t start_page = kStartOfText - kStartOfText % kPageSize;
// Set the end to the page on which the beginning of the last symbol is. The
// actual symbol may spill into the next page by a few bytes, but this is
// outside of the executable code range anyway.
size_t end_page = base::bits::Align(kEndOfText, kPageSize);
return {start_page, end_page};
}

// Populate the per-page residency for |range| in |residency|. If successful,
// |residency| has the size of |range| in pages.
// Returns true for success.
Expand All @@ -95,11 +110,8 @@ bool MincoreOnRange(const NativeLibraryPrefetcher::AddressRange& range,
residency->resize(size_in_pages);
int err = HANDLE_EINTR(
mincore(reinterpret_cast<void*>(range.first), size, &(*residency)[0]));
if (err) {
PLOG(ERROR) << "mincore() failed";
return false;
}
return true;
PLOG_IF(ERROR, err) << "mincore() failed";
return !err;
}

// Timestamp in ns since Unix Epoch, and residency, as returned by mincore().
Expand Down Expand Up @@ -278,28 +290,27 @@ int NativeLibraryPrefetcher::PercentageOfResidentNativeLibraryCode() {
// static
void NativeLibraryPrefetcher::PeriodicallyCollectResidency() {
CHECK_EQ(static_cast<long>(kPageSize), sysconf(_SC_PAGESIZE));
std::vector<AddressRange> ranges;
if (!FindRanges(&ranges))
return;

// To keep only the range containing .text, find out which one contains
// ourself.
const size_t here = reinterpret_cast<size_t>(
&NativeLibraryPrefetcher::PeriodicallyCollectResidency);
auto it =
std::find_if(ranges.begin(), ranges.end(), [here](const AddressRange& r) {
return r.first <= here && here <= r.second;
});
CHECK(ranges.end() != it);

const auto& range = GetTextRange();
auto data = std::make_unique<std::vector<TimestampAndResidency>>();
for (int i = 0; i < 60; ++i) {
if (!CollectResidency(*it, data.get()))
if (!CollectResidency(range, data.get()))
return;
usleep(2e5);
}
DumpResidency(std::move(data));
}

// static
void NativeLibraryPrefetcher::MadviseRandomText() {
CheckOrderingSanity();
const auto& range = GetTextRange();
size_t size = range.second - range.first;
int err = madvise(reinterpret_cast<void*>(range.first), size, MADV_RANDOM);
if (err) {
PLOG(ERROR) << "madvise() failed";
}
}

} // namespace android
} // namespace base
3 changes: 3 additions & 0 deletions base/android/library_loader/library_prefetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class BASE_EXPORT NativeLibraryPrefetcher {
// dumps it to disk.
static void PeriodicallyCollectResidency();

// Calls madvise(MADV_RANDOM) on the native library executable code range.
static void MadviseRandomText();

private:
// Returns true if the region matches native code or data.
static bool IsGoodToPrefetch(const base::debug::MappedMemoryRegion& region);
Expand Down
6 changes: 6 additions & 0 deletions base/base_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,10 @@ const char kEnableCrashReporterForTesting[] =
"enable-crash-reporter-for-testing";
#endif

#if defined(OS_ANDROID)
// Calls madvise(MADV_RANDOM) on executable code right after the library is
// loaded, from all processes.
const char kMadviseRandomExecutableCode[] = "madvise-random-executable-code";
#endif

} // namespace switches
4 changes: 4 additions & 0 deletions base/base_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ extern const char kDisableUsbKeyboardDetect[];
extern const char kEnableCrashReporterForTesting[];
#endif

#if defined(OS_ANDROID)
extern const char kMadviseRandomExecutableCode[];
#endif

} // namespace switches

#endif // BASE_BASE_SWITCHES_H_
3 changes: 3 additions & 0 deletions content/browser/gpu/gpu_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ static const char* const kSwitchNames[] = {
switches::kUseCmdDecoder,
switches::kIgnoreGpuBlacklist,
switches::kForceVideoOverlays,
#if defined(OS_ANDROID)
switches::kMadviseRandomExecutableCode,
#endif
};

enum GPUProcessLifetimeEvent {
Expand Down
1 change: 1 addition & 0 deletions content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2687,6 +2687,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
switches::kDisallowNonExactResourceReuse,
#if defined(OS_ANDROID)
switches::kDisableMediaSessionAPI,
switches::kMadviseRandomExecutableCode,
switches::kRendererWaitForJavaDebugger,
#endif
#if defined(OS_MACOSX)
Expand Down
5 changes: 4 additions & 1 deletion content/browser/utility_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,10 @@ bool UtilityProcessHostImpl::StartProcess() {
switches::kForceMediaFoundationVideoCapture,
#endif // defined(OS_WIN)
switches::kUtilityStartupDialog,
switches::kUseGL
switches::kUseGL,
#if defined(OS_ANDROID)
switches::kMadviseRandomExecutableCode,
#endif
};
cmd_line->CopySwitchesFrom(browser_command_line, kSwitchNames,
arraysize(kSwitchNames));
Expand Down

0 comments on commit 411a34f

Please sign in to comment.