Reland^2 "Always put the framework profile in the boot image location."
This reverts commit acb5c2dfabaf8126e20d17a4d02ac183af2afc26.
And adjust dex2oat image location adjusting logic, by removing the
profile, but keeping the extension location.
Bug: 143126914
Bug: 119800099
Bug: 149859910
Bug: 150193586
Reason for revert: Changes in dex2oat.cc fix the performance regression.
Change-Id: I4686d2af029d7185c02d7e798f19d0f5d9328dbc
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index a71dcb4..99c6bd4 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1045,7 +1045,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.
@@ -1053,15 +1059,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();