Delay dex-to-dex compilation until Optimizing is done.
This fixes a race between inlining in the Optimizing
backend and dex-to-dex quickening where the Optimizing can
read the non-quickened opcode and then the quickened field
index or vtable index and look up the wrong field or method.
Even if we such tearing of the dex instruction does not
happen, the possible reordering of dex-to-dex and Optimizing
compilation makes the final oat file non-deterministic.
Also, remove VerificationResults::RemoveVerifiedMethod() as
we have only the Optimizing backend now and as such it was
dead code and would have interfered with this change.
Bug: 29043547
Bug: 29089975
(cherry picked from commit 492a7fa6df3b197a24099a50f5abf624164f3842)
Change-Id: I1337b772dc69318393845a790e5f6d38aa3de60f
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 4594e2c..e366e07 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -26,6 +26,7 @@
#include "art_field-inl.h"
#include "art_method-inl.h"
+#include "base/bit_vector.h"
#include "base/stl_util.h"
#include "base/systrace.h"
#include "base/time_utils.h"
@@ -66,6 +67,7 @@
#include "thread_pool.h"
#include "trampolines/trampoline_compiler.h"
#include "transaction.h"
+#include "utils/array_ref.h"
#include "utils/dex_cache_arrays_layout-inl.h"
#include "utils/swap_space.h"
#include "verifier/method_verifier.h"
@@ -333,6 +335,24 @@
DISALLOW_COPY_AND_ASSIGN(AOTCompilationStats);
};
+class CompilerDriver::DexFileMethodSet {
+ public:
+ explicit DexFileMethodSet(const DexFile& dex_file)
+ : dex_file_(dex_file),
+ method_indexes_(dex_file.NumMethodIds(), false, Allocator::GetMallocAllocator()) {
+ }
+ DexFileMethodSet(DexFileMethodSet&& other) = default;
+
+ const DexFile& GetDexFile() const { return dex_file_; }
+
+ BitVector& GetMethodIndexes() { return method_indexes_; }
+ const BitVector& GetMethodIndexes() const { return method_indexes_; }
+
+ private:
+ const DexFile& dex_file_;
+ BitVector method_indexes_;
+};
+
CompilerDriver::CompilerDriver(
const CompilerOptions* compiler_options,
VerificationResults* verification_results,
@@ -379,7 +399,10 @@
dex_files_for_oat_file_(nullptr),
compiled_method_storage_(swap_fd),
profile_compilation_info_(profile_compilation_info),
- max_arena_alloc_(0) {
+ max_arena_alloc_(0),
+ dex_to_dex_references_lock_("dex-to-dex references lock"),
+ dex_to_dex_references_(),
+ current_dex_to_dex_methods_(nullptr) {
DCHECK(compiler_options_ != nullptr);
DCHECK(method_inliner_map_ != nullptr);
@@ -552,7 +575,29 @@
uint64_t start_ns = kTimeCompileMethod ? NanoTime() : 0;
MethodReference method_ref(&dex_file, method_idx);
- if ((access_flags & kAccNative) != 0) {
+ if (driver->GetCurrentDexToDexMethods() != nullptr) {
+ // This is the second pass when we dex-to-dex compile previously marked methods.
+ // TODO: Refactor the compilation to avoid having to distinguish the two passes
+ // here. That should be done on a higher level. http://b/29089975
+ if (driver->GetCurrentDexToDexMethods()->IsBitSet(method_idx)) {
+ const VerifiedMethod* verified_method =
+ driver->GetVerificationResults()->GetVerifiedMethod(method_ref);
+ // Do not optimize if a VerifiedMethod is missing. SafeCast elision,
+ // for example, relies on it.
+ compiled_method = optimizer::ArtCompileDEX(
+ driver,
+ code_item,
+ access_flags,
+ invoke_type,
+ class_def_idx,
+ method_idx,
+ class_loader,
+ dex_file,
+ (verified_method != nullptr)
+ ? dex_to_dex_compilation_level
+ : optimizer::DexToDexCompilationLevel::kRequired);
+ }
+ } else if ((access_flags & kAccNative) != 0) {
// Are we extracting only and have support for generic JNI down calls?
if (!driver->GetCompilerOptions().IsJniCompilationEnabled() &&
InstructionSetHasGenericJniStub(driver->GetInstructionSet())) {
@@ -588,21 +633,9 @@
}
if (compiled_method == nullptr &&
dex_to_dex_compilation_level != optimizer::DexToDexCompilationLevel::kDontDexToDexCompile) {
+ DCHECK(!Runtime::Current()->UseJitCompilation());
// TODO: add a command-line option to disable DEX-to-DEX compilation ?
- // Do not optimize if a VerifiedMethod is missing. SafeCast elision, for example, relies on
- // it.
- compiled_method = optimizer::ArtCompileDEX(
- driver,
- code_item,
- access_flags,
- invoke_type,
- class_def_idx,
- method_idx,
- class_loader,
- dex_file,
- (verified_method != nullptr)
- ? dex_to_dex_compilation_level
- : optimizer::DexToDexCompilationLevel::kRequired);
+ driver->MarkForDexToDexCompilation(self, method_ref);
}
}
if (kTimeCompileMethod) {
@@ -628,12 +661,6 @@
driver->AddCompiledMethod(method_ref, compiled_method, non_relative_linker_patch_count);
}
- // Done compiling, delete the verified method to reduce native memory usage. Do not delete in
- // optimizing compiler, which may need the verified method again for inlining.
- if (driver->GetCompilerKind() != Compiler::kOptimizing) {
- driver->GetVerificationResults()->RemoveVerifiedMethod(method_ref);
- }
-
if (self->IsExceptionPending()) {
ScopedObjectAccess soa(self);
LOG(FATAL) << "Unexpected exception compiling: " << PrettyMethod(method_idx, dex_file) << "\n"
@@ -680,6 +707,7 @@
*dex_file,
dex_file->GetClassDef(class_def_idx));
+ DCHECK(current_dex_to_dex_methods_ == nullptr);
CompileMethod(self,
this,
code_item,
@@ -693,6 +721,34 @@
true,
dex_cache);
+ ArrayRef<DexFileMethodSet> dex_to_dex_references;
+ {
+ // From this point on, we shall not modify dex_to_dex_references_, so
+ // just grab a reference to it that we use without holding the mutex.
+ MutexLock lock(Thread::Current(), dex_to_dex_references_lock_);
+ dex_to_dex_references = ArrayRef<DexFileMethodSet>(dex_to_dex_references_);
+ }
+ if (!dex_to_dex_references.empty()) {
+ DCHECK_EQ(dex_to_dex_references.size(), 1u);
+ DCHECK(&dex_to_dex_references[0].GetDexFile() == dex_file);
+ current_dex_to_dex_methods_ = &dex_to_dex_references.front().GetMethodIndexes();
+ DCHECK(current_dex_to_dex_methods_->IsBitSet(method_idx));
+ DCHECK_EQ(current_dex_to_dex_methods_->NumSetBits(), 1u);
+ CompileMethod(self,
+ this,
+ code_item,
+ access_flags,
+ invoke_type,
+ class_def_idx,
+ method_idx,
+ jclass_loader,
+ *dex_file,
+ dex_to_dex_compilation_level,
+ true,
+ dex_cache);
+ current_dex_to_dex_methods_ = nullptr;
+ }
+
FreeThreadPools();
self->GetJniEnv()->DeleteGlobalRef(jclass_loader);
@@ -1285,6 +1341,17 @@
return IsImageClass(descriptor);
}
+void CompilerDriver::MarkForDexToDexCompilation(Thread* self, const MethodReference& method_ref) {
+ MutexLock lock(self, dex_to_dex_references_lock_);
+ // Since we're compiling one dex file at a time, we need to look for the
+ // current dex file entry only at the end of dex_to_dex_references_.
+ if (dex_to_dex_references_.empty() ||
+ &dex_to_dex_references_.back().GetDexFile() != method_ref.dex_file) {
+ dex_to_dex_references_.emplace_back(*method_ref.dex_file);
+ }
+ dex_to_dex_references_.back().GetMethodIndexes().SetBit(method_ref.dex_method_index);
+}
+
bool CompilerDriver::CanAssumeTypeIsPresentInDexCache(Handle<mirror::DexCache> dex_cache,
uint32_t type_idx) {
bool result = false;
@@ -2496,8 +2563,9 @@
? "null"
: profile_compilation_info_->DumpInfo(&dex_files));
}
- for (size_t i = 0; i != dex_files.size(); ++i) {
- const DexFile* dex_file = dex_files[i];
+
+ DCHECK(current_dex_to_dex_methods_ == nullptr);
+ for (const DexFile* dex_file : dex_files) {
CHECK(dex_file != nullptr);
CompileDexFile(class_loader,
*dex_file,
@@ -2510,6 +2578,25 @@
max_arena_alloc_ = std::max(arena_alloc, max_arena_alloc_);
Runtime::Current()->ReclaimArenaPoolMemory();
}
+
+ ArrayRef<DexFileMethodSet> dex_to_dex_references;
+ {
+ // From this point on, we shall not modify dex_to_dex_references_, so
+ // just grab a reference to it that we use without holding the mutex.
+ MutexLock lock(Thread::Current(), dex_to_dex_references_lock_);
+ dex_to_dex_references = ArrayRef<DexFileMethodSet>(dex_to_dex_references_);
+ }
+ for (const auto& method_set : dex_to_dex_references) {
+ current_dex_to_dex_methods_ = &method_set.GetMethodIndexes();
+ CompileDexFile(class_loader,
+ method_set.GetDexFile(),
+ dex_files,
+ parallel_thread_pool_.get(),
+ parallel_thread_count_,
+ timings);
+ }
+ current_dex_to_dex_methods_ = nullptr;
+
VLOG(compiler) << "Compile: " << GetMemoryUsageString(false);
}