Reland^3 "Always put the framework profile in the boot image location."
This reverts commit 9bdba6e5e252de50c5cdd7f1680b84d87c903115.
Bug: 143126914
Bug: 119800099
Bug: 149859910
Bug: 150193586
Bug: 150890582
Reason for revert: CL not the reason for the regression.
Change-Id: I4d6b2c1dde87930054f15c5a466275f255f81302
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index a1b6cc7..5e3743f 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1109,7 +1109,13 @@
}
}
- // Trim the boot image location to the components that do not have any profile specified.
+ // Trim the boot image location to not include any specified profile. Note
+ // that the logic below will include the first boot image extension, but not
+ // the ones that could be listed after the profile of that extension. This
+ // works for our current top use case:
+ // boot.art:/system/framework/boot-framework.art
+ // But this would need to be adjusted if we had to support different use
+ // cases.
size_t profile_separator_pos = boot_image_filename_.find(ImageSpace::kProfileSeparator);
if (profile_separator_pos != std::string::npos) {
DCHECK(!IsBootImage()); // For primary boot image the boot_image_filename_ is empty.
@@ -1117,15 +1123,11 @@
Usage("Unsupported profile specification in boot image location (%s) for extension.",
boot_image_filename_.c_str());
}
- size_t component_separator_pos =
- boot_image_filename_.rfind(ImageSpace::kComponentSeparator, profile_separator_pos);
- if (component_separator_pos != std::string::npos && component_separator_pos != 0u) {
- VLOG(compiler)
- << "Truncating boot image location " << boot_image_filename_
- << " because it contains profile specification. Truncated: "
- << boot_image_filename_.substr(/*pos*/ 0u, /*length*/ component_separator_pos);
- boot_image_filename_.resize(component_separator_pos);
- } // else let the full validation in ImageSpace reject the invalid location.
+ VLOG(compiler)
+ << "Truncating boot image location " << boot_image_filename_
+ << " because it contains profile specification. Truncated: "
+ << boot_image_filename_.substr(/*pos*/ 0u, /*length*/ profile_separator_pos);
+ boot_image_filename_.resize(profile_separator_pos);
}
compiler_options_->passes_to_run_ = passes_to_run_.get();
diff --git a/libartbase/base/file_utils.cc b/libartbase/base/file_utils.cc
index 3fd1980..a899b86 100644
--- a/libartbase/base/file_utils.cc
+++ b/libartbase/base/file_utils.cc
@@ -278,8 +278,9 @@
// Boot image consists of two parts:
// - the primary boot image in the ART apex (contains the Core Libraries)
// - the boot image extension on the system partition (contains framework libraries)
- return StringPrintf("%s/javalib/boot.art:%s/framework/boot-framework.art",
+ return StringPrintf("%s/javalib/boot.art:%s/framework/boot-framework.art!%s/etc/boot-image.prof",
kAndroidArtApexDefaultPath,
+ android_root.c_str(),
android_root.c_str());
}
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index 1990457..8ae57ff 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -767,7 +767,10 @@
ScopedObjectAccess soa(Thread::Current());
// For a non-bootclasspath class, add a global ref to the class to prevent class unloading
// until compilation is done.
- if (method->GetDeclaringClass()->GetClassLoader() != nullptr) {
+ // When we precompile, this is either with boot classpath methods, or main
+ // class loader methods, so we don't need to keep a global reference.
+ if (method->GetDeclaringClass()->GetClassLoader() != nullptr &&
+ kind_ != TaskKind::kPreCompile) {
klass_ = soa.Vm()->AddGlobalRef(soa.Self(), method_->GetDeclaringClass());
CHECK(klass_ != nullptr);
}
@@ -1116,6 +1119,17 @@
LOG(INFO) << "Successfully mapped boot image methods";
}
+// Return whether a boot image has a profile. This means we'll need to pre-JIT
+// methods in that profile for performance.
+static bool HasImageWithProfile() {
+ for (gc::space::ImageSpace* space : Runtime::Current()->GetHeap()->GetBootImageSpaces()) {
+ if (!space->GetProfileFile().empty()) {
+ return true;
+ }
+ }
+ return false;
+}
+
void Jit::CreateThreadPool() {
// There is a DCHECK in the 'AddSamples' method to ensure the tread pool
// is not null when we instrument.
@@ -1128,9 +1142,9 @@
Start();
Runtime* runtime = Runtime::Current();
- if (runtime->IsZygote() && runtime->IsRunningJitZygote() && UseJitCompilation()) {
- // If we're not using the default boot image location, request a JIT task to
- // compile all methods in the boot image profile.
+ if (runtime->IsZygote() && HasImageWithProfile() && UseJitCompilation()) {
+ // If we have an image with a profile, request a JIT task to
+ // compile all methods in that profile.
thread_pool_->AddTask(Thread::Current(), new ZygoteTask());
// And create mappings to share boot image methods memory from the zygote to
@@ -1209,7 +1223,7 @@
return;
}
Runtime* runtime = Runtime::Current();
- if (runtime->IsSystemServer() && runtime->IsRunningJitZygote() && UseJitCompilation()) {
+ if (runtime->IsSystemServer() && UseJitCompilation() && HasImageWithProfile()) {
thread_pool_->AddTask(Thread::Current(), new JitProfileTask(dex_files, class_loader));
}
}
@@ -1237,7 +1251,9 @@
const void* entry_point = method->GetEntryPointFromQuickCompiledCode();
if (class_linker->IsQuickToInterpreterBridge(entry_point) ||
class_linker->IsQuickGenericJniStub(entry_point) ||
- class_linker->IsQuickResolutionStub(entry_point)) {
+ // We explicitly check for the stub. The trampoline is for methods backed by
+ // a .oat file that has a compiled version of the method.
+ (entry_point == GetQuickResolutionStub())) {
method->SetPreCompiled();
if (!add_to_queue) {
CompileMethod(method, self, /* baseline= */ false, /* osr= */ false, /* prejit= */ true);
@@ -1463,13 +1479,6 @@
}
}
if (UseJitCompilation()) {
- if (old_count == 0 &&
- method->IsNative() &&
- Runtime::Current()->IsRunningJitZygote()) {
- // jitzygote: Compile JNI stub on first use to avoid the expensive generic stub.
- CompileMethod(method, self, /* baseline= */ false, /* osr= */ false, /* prejit= */ false);
- return true;
- }
if (old_count < HotMethodThreshold() && new_count >= HotMethodThreshold()) {
if (!code_cache_->ContainsPc(method->GetEntryPointFromQuickCompiledCode())) {
DCHECK(thread_pool_ != nullptr);
@@ -1639,10 +1648,11 @@
}
Runtime* const runtime = Runtime::Current();
- // For child zygote, we instead query IsCompilationNotified() post zygote fork.
- if (!is_zygote && runtime->IsRunningJitZygote() && fd_methods_ != -1) {
+ // Check if we'll need to remap the boot image methods.
+ if (!is_zygote && fd_methods_ != -1) {
// Create a thread that will poll the status of zygote compilation, and map
// the private mapping of boot image methods.
+ // For child zygote, we instead query IsCompilationNotified() post zygote fork.
zygote_mapping_methods_.ResetInForkedProcess();
pthread_t polling_thread;
pthread_attr_t attr;
@@ -1668,7 +1678,7 @@
code_cache_->SetGarbageCollectCode(!jit_compiler_->GenerateDebugInfo() &&
!Runtime::Current()->GetInstrumentation()->AreExitStubsInstalled());
- if (is_system_server && Runtime::Current()->IsRunningJitZygote()) {
+ if (is_system_server && HasImageWithProfile()) {
// Disable garbage collection: we don't want it to delete methods we're compiling
// through boot and system server profiles.
// TODO(ngeoffray): Fix this so we still collect deoptimized and unused code.
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 3e618fa..655f05f 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -979,11 +979,6 @@
// Return true if we should load oat files as executable or not.
bool GetOatFilesExecutable() const;
- bool IsRunningJitZygote() const {
- // TODO: This should be better specified.
- return EndsWith(image_location_, "boot-image.prof");
- }
-
private:
static void InitPlatformSignalHandlers();