Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Commit

Permalink
Delay dex-to-dex compilation until Optimizing is done.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@vmarko
vmarko committed Jun 3, 2016
1 parent 2dc77ec commit b089ecc
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 37 deletions.
9 changes: 0 additions & 9 deletions compiler/dex/verification_results.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,6 @@ const VerifiedMethod* VerificationResults::GetVerifiedMethod(MethodReference ref
return (it != verified_methods_.end()) ? it->second : nullptr;
}

void VerificationResults::RemoveVerifiedMethod(MethodReference ref) {
WriterMutexLock mu(Thread::Current(), verified_methods_lock_);
auto it = verified_methods_.find(ref);
if (it != verified_methods_.end()) {
delete it->second;
verified_methods_.erase(it);
}
}

void VerificationResults::AddRejectedClass(ClassReference ref) {
{
WriterMutexLock mu(Thread::Current(), rejected_classes_lock_);
Expand Down
1 change: 0 additions & 1 deletion compiler/dex/verification_results.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class VerificationResults {

const VerifiedMethod* GetVerifiedMethod(MethodReference ref)
REQUIRES(!verified_methods_lock_);
void RemoveVerifiedMethod(MethodReference ref) REQUIRES(!verified_methods_lock_);

void AddRejectedClass(ClassReference ref) REQUIRES(!rejected_classes_lock_);
bool IsClassRejected(ClassReference ref) REQUIRES(!rejected_classes_lock_);
Expand Down
135 changes: 111 additions & 24 deletions compiler/driver/compiler_driver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -333,6 +335,24 @@ class CompilerDriver::AOTCompilationStats {
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,
Expand Down Expand Up @@ -379,7 +399,10 @@ CompilerDriver::CompilerDriver(
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);

Expand Down Expand Up @@ -552,7 +575,29 @@ static void CompileMethod(Thread* self,
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())) {
Expand Down Expand Up @@ -588,21 +633,9 @@ static void CompileMethod(Thread* self,
}
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) {
Expand All @@ -628,12 +661,6 @@ static void CompileMethod(Thread* self,
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"
Expand Down Expand Up @@ -680,6 +707,7 @@ void CompilerDriver::CompileOne(Thread* self, ArtMethod* method, TimingLogger* t
*dex_file,
dex_file->GetClassDef(class_def_idx));

DCHECK(current_dex_to_dex_methods_ == nullptr);
CompileMethod(self,
this,
code_item,
Expand All @@ -693,6 +721,34 @@ void CompilerDriver::CompileOne(Thread* self, ArtMethod* method, TimingLogger* t
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);
Expand Down Expand Up @@ -1285,6 +1341,17 @@ bool CompilerDriver::CanAssumeClassIsLoaded(mirror::Class* klass) {
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;
Expand Down Expand Up @@ -2496,8 +2563,9 @@ void CompilerDriver::Compile(jobject class_loader,
? "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,
Expand All @@ -2510,6 +2578,25 @@ void CompilerDriver::Compile(jobject class_loader,
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);
}

Expand Down
24 changes: 21 additions & 3 deletions compiler/driver/compiler_driver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ namespace verifier {
class MethodVerifier;
} // namespace verifier

class BitVector;
class CompiledClass;
class CompiledMethod;
class CompilerOptions;
Expand Down Expand Up @@ -120,12 +121,12 @@ class CompilerDriver {
void CompileAll(jobject class_loader,
const std::vector<const DexFile*>& dex_files,
TimingLogger* timings)
REQUIRES(!Locks::mutator_lock_, !compiled_classes_lock_);
REQUIRES(!Locks::mutator_lock_, !compiled_classes_lock_, !dex_to_dex_references_lock_);

// Compile a single Method.
void CompileOne(Thread* self, ArtMethod* method, TimingLogger* timings)
SHARED_REQUIRES(Locks::mutator_lock_)
REQUIRES(!compiled_methods_lock_, !compiled_classes_lock_);
REQUIRES(!compiled_methods_lock_, !compiled_classes_lock_, !dex_to_dex_references_lock_);

VerificationResults* GetVerificationResults() const {
DCHECK(Runtime::Current()->IsAotCompiler());
Expand Down Expand Up @@ -475,6 +476,13 @@ class CompilerDriver {
return true;
}

void MarkForDexToDexCompilation(Thread* self, const MethodReference& method_ref)
REQUIRES(!dex_to_dex_references_lock_);

const BitVector* GetCurrentDexToDexMethods() const {
return current_dex_to_dex_methods_;
}

private:
// Return whether the declaring class of `resolved_member` is
// available to `referrer_class` for read or write access using two
Expand Down Expand Up @@ -601,7 +609,7 @@ class CompilerDriver {

void Compile(jobject class_loader,
const std::vector<const DexFile*>& dex_files,
TimingLogger* timings);
TimingLogger* timings) REQUIRES(!dex_to_dex_references_lock_);
void CompileDexFile(jobject class_loader,
const DexFile& dex_file,
const std::vector<const DexFile*>& dex_files,
Expand Down Expand Up @@ -702,6 +710,16 @@ class CompilerDriver {
const ProfileCompilationInfo* const profile_compilation_info_;

size_t max_arena_alloc_;

// Data for delaying dex-to-dex compilation.
Mutex dex_to_dex_references_lock_;
// In the first phase, dex_to_dex_references_ collects methods for dex-to-dex compilation.
class DexFileMethodSet;
std::vector<DexFileMethodSet> dex_to_dex_references_ GUARDED_BY(dex_to_dex_references_lock_);
// In the second phase, current_dex_to_dex_methods_ points to the BitVector with method
// indexes for dex-to-dex compilation in the current dex file.
const BitVector* current_dex_to_dex_methods_;

friend class CompileClassVisitor;
DISALLOW_COPY_AND_ASSIGN(CompilerDriver);
};
Expand Down
14 changes: 14 additions & 0 deletions runtime/base/bit_vector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ class BitVector {
const BitVector* const bit_vector_;
};

// MoveConstructible but not MoveAssignable, CopyConstructible or CopyAssignable.

BitVector(const BitVector& other) = delete;
BitVector& operator=(const BitVector& other) = delete;

BitVector(BitVector&& other)
: storage_(other.storage_),
storage_size_(other.storage_size_),
allocator_(other.allocator_),
expandable_(other.expandable_) {
other.storage_ = nullptr;
other.storage_size_ = 0u;
}

BitVector(uint32_t start_bits,
bool expandable,
Allocator* allocator);
Expand Down

0 comments on commit b089ecc

Please sign in to comment.