diff options
| -rw-r--r-- | runtime/base/mutex.cc | 8 | ||||
| -rw-r--r-- | runtime/base/mutex.h | 12 | ||||
| -rw-r--r-- | runtime/cha.cc | 71 | ||||
| -rw-r--r-- | runtime/jit/jit_code_cache.cc | 111 | ||||
| -rw-r--r-- | runtime/jit/jit_code_cache.h | 6 |
5 files changed, 91 insertions, 117 deletions
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc index 044c4c2f78..7b888b18d9 100644 --- a/runtime/base/mutex.cc +++ b/runtime/base/mutex.cc @@ -1142,6 +1142,10 @@ void Locks::Init() { DCHECK(subtype_check_lock_ == nullptr); subtype_check_lock_ = new Mutex("SubtypeCheck lock", current_lock_level); + UPDATE_CURRENT_LOCK_LEVEL(kCHALock); + DCHECK(cha_lock_ == nullptr); + cha_lock_ = new Mutex("CHA lock", current_lock_level); + UPDATE_CURRENT_LOCK_LEVEL(kClassLinkerClassesLock); DCHECK(classlinker_classes_lock_ == nullptr); classlinker_classes_lock_ = new ReaderWriterMutex("ClassLinker classes lock", @@ -1222,10 +1226,6 @@ void Locks::Init() { DCHECK(custom_tls_lock_ == nullptr); custom_tls_lock_ = new Mutex("Thread::custom_tls_ lock", current_lock_level); - UPDATE_CURRENT_LOCK_LEVEL(kCHALock); - DCHECK(cha_lock_ == nullptr); - cha_lock_ = new Mutex("CHA lock", current_lock_level); - UPDATE_CURRENT_LOCK_LEVEL(kNativeDebugInterfaceLock); DCHECK(native_debug_interface_lock_ == nullptr); native_debug_interface_lock_ = new Mutex("Native debug interface lock", current_lock_level); diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h index fba209a0b6..af2e7b2763 100644 --- a/runtime/base/mutex.h +++ b/runtime/base/mutex.h @@ -72,7 +72,6 @@ enum LockLevel : uint8_t { kJdwpSocketLock, kRegionSpaceRegionLock, kMarkSweepMarkStackLock, - kCHALock, kJitCodeCacheLock, kRosAllocGlobalLock, kRosAllocBracketLock, @@ -110,6 +109,7 @@ enum LockLevel : uint8_t { kMonitorPoolLock, kClassLinkerClassesLock, // TODO rename. kDexToDexCompilerLock, + kCHALock, kSubtypeCheckLock, kBreakpointLock, kMonitorLock, @@ -661,11 +661,14 @@ class Locks { // TODO: improve name, perhaps instrumentation_update_lock_. static Mutex* deoptimization_lock_ ACQUIRED_AFTER(alloc_tracker_lock_); + // Guards Class Hierarchy Analysis (CHA). + static Mutex* cha_lock_ ACQUIRED_AFTER(deoptimization_lock_); + // Guard the update of the SubtypeCheck data stores in each Class::status_ field. // This lock is used in SubtypeCheck methods which are the interface for // any SubtypeCheck-mutating methods. // In Class::IsSubClass, the lock is not required since it does not update the SubtypeCheck data. - static Mutex* subtype_check_lock_ ACQUIRED_AFTER(deoptimization_lock_); + static Mutex* subtype_check_lock_ ACQUIRED_AFTER(cha_lock_); // The thread_list_lock_ guards ThreadList::list_. It is also commonly held to stop threads // attaching and detaching. @@ -742,14 +745,11 @@ class Locks { // GetThreadLocalStorage. static Mutex* custom_tls_lock_ ACQUIRED_AFTER(jni_function_table_lock_); - // Guards Class Hierarchy Analysis (CHA). - static Mutex* cha_lock_ ACQUIRED_AFTER(custom_tls_lock_); - // When declaring any Mutex add BOTTOM_MUTEX_ACQUIRED_AFTER to use annotalysis to check the code // doesn't try to acquire a higher level Mutex. NB Due to the way the annotalysis works this // actually only encodes the mutex being below jni_function_table_lock_ although having // kGenericBottomLock level is lower than this. - #define BOTTOM_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(art::Locks::cha_lock_) + #define BOTTOM_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(art::Locks::custom_tls_lock_) // Have an exclusive aborting thread. static Mutex* abort_lock_ ACQUIRED_AFTER(custom_tls_lock_); diff --git a/runtime/cha.cc b/runtime/cha.cc index 6354763ea9..ccbe066ed6 100644 --- a/runtime/cha.cc +++ b/runtime/cha.cc @@ -636,51 +636,38 @@ void ClassHierarchyAnalysis::InvalidateSingleImplementationMethods( // We do this under cha_lock_. Committing code also grabs this lock to // make sure the code is only committed when all single-implementation // assumptions are still true. - std::vector<std::pair<ArtMethod*, OatQuickMethodHeader*>> headers; - { - MutexLock cha_mu(self, *Locks::cha_lock_); - // Invalidate compiled methods that assume some virtual calls have only - // single implementations. - for (ArtMethod* invalidated : invalidated_single_impl_methods) { - if (!invalidated->HasSingleImplementation()) { - // It might have been invalidated already when other class linking is - // going on. - continue; - } - invalidated->SetHasSingleImplementation(false); - if (invalidated->IsAbstract()) { - // Clear the single implementation method. - invalidated->SetSingleImplementation(nullptr, image_pointer_size); - } + MutexLock cha_mu(self, *Locks::cha_lock_); + // Invalidate compiled methods that assume some virtual calls have only + // single implementations. + for (ArtMethod* invalidated : invalidated_single_impl_methods) { + if (!invalidated->HasSingleImplementation()) { + // It might have been invalidated already when other class linking is + // going on. + continue; + } + invalidated->SetHasSingleImplementation(false); + if (invalidated->IsAbstract()) { + // Clear the single implementation method. + invalidated->SetSingleImplementation(nullptr, image_pointer_size); + } - if (runtime->IsAotCompiler()) { - // No need to invalidate any compiled code as the AotCompiler doesn't - // run any code. - continue; - } + if (runtime->IsAotCompiler()) { + // No need to invalidate any compiled code as the AotCompiler doesn't + // run any code. + continue; + } - // Invalidate all dependents. - for (const auto& dependent : GetDependents(invalidated)) { - ArtMethod* method = dependent.first;; - OatQuickMethodHeader* method_header = dependent.second; - VLOG(class_linker) << "CHA invalidated compiled code for " << method->PrettyMethod(); - DCHECK(runtime->UseJitCompilation()); - // We need to call JitCodeCache::InvalidateCompiledCodeFor but we cannot do it here - // since it would run into problems with lock-ordering. We don't want to re-order the - // locks since that would make code-commit racy. - headers.push_back({method, method_header}); - dependent_method_headers.insert(method_header); - } - RemoveAllDependenciesFor(invalidated); + // Invalidate all dependents. + for (const auto& dependent : GetDependents(invalidated)) { + ArtMethod* method = dependent.first;; + OatQuickMethodHeader* method_header = dependent.second; + VLOG(class_linker) << "CHA invalidated compiled code for " << method->PrettyMethod(); + DCHECK(runtime->UseJitCompilation()); + runtime->GetJit()->GetCodeCache()->InvalidateCompiledCodeFor( + method, method_header); + dependent_method_headers.insert(method_header); } - } - // Since we are still loading the class that invalidated the code it's fine we have this after - // getting rid of the dependency. Any calls would need to be with the old version (since the - // new one isn't loaded yet) which still works fine. We will deoptimize just after this to - // ensure everything gets the new state. - jit::JitCodeCache* code_cache = Runtime::Current()->GetJit()->GetCodeCache(); - for (const auto& pair : headers) { - code_cache->InvalidateCompiledCodeFor(pair.first, pair.second); + RemoveAllDependenciesFor(invalidated); } } diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc index c1055630f7..b92affa26e 100644 --- a/runtime/jit/jit_code_cache.cc +++ b/runtime/jit/jit_code_cache.cc @@ -608,17 +608,17 @@ void JitCodeCache::FreeCodeAndData(const void* code_ptr) { void JitCodeCache::FreeAllMethodHeaders( const std::unordered_set<OatQuickMethodHeader*>& method_headers) { - // We need to remove entries in method_headers from CHA dependencies - // first since once we do FreeCode() below, the memory can be reused - // so it's possible for the same method_header to start representing - // different compile code. - MutexLock mu(Thread::Current(), lock_); { - MutexLock mu2(Thread::Current(), *Locks::cha_lock_); + MutexLock mu(Thread::Current(), *Locks::cha_lock_); Runtime::Current()->GetClassLinker()->GetClassHierarchyAnalysis() ->RemoveDependentsWithMethodHeaders(method_headers); } + // We need to remove entries in method_headers from CHA dependencies + // first since once we do FreeCode() below, the memory can be reused + // so it's possible for the same method_header to start representing + // different compile code. + MutexLock mu(Thread::Current(), lock_); ScopedCodeCacheWrite scc(this); for (const OatQuickMethodHeader* method_header : method_headers) { FreeCodeAndData(method_header->GetCode()); @@ -742,20 +742,6 @@ static void ClearMethodCounter(ArtMethod* method, bool was_warm) { method->SetCounter(std::min(jit_warmup_threshold - 1, 1)); } -bool JitCodeCache::WaitForPotentialCollectionToCompleteRunnable(Thread* self) { - bool waited = false; - while (collection_in_progress_) { - lock_.Unlock(self); - { - ScopedThreadSuspension sts(self, kSuspended); - MutexLock mu(self, lock_); - waited = WaitForPotentialCollectionToComplete(self); - } - lock_.Lock(self); - } - return waited; -} - uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, ArtMethod* method, uint8_t* stack_map, @@ -769,13 +755,6 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, const ArenaSet<ArtMethod*>& cha_single_implementation_list) { DCHECK(!method->IsNative() || !osr); - - if (!method->IsNative()) { - // We need to do this before grabbing the lock_ because it needs to be able to see the string - // InternTable. Native methods do not have roots. - DCheckRootsAreValid(roots); - } - size_t alignment = GetInstructionSetAlignment(kRuntimeISA); // Ensure the header ends up at expected instruction alignment. size_t header_size = RoundUp(sizeof(OatQuickMethodHeader), alignment); @@ -784,45 +763,44 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, OatQuickMethodHeader* method_header = nullptr; uint8_t* code_ptr = nullptr; uint8_t* memory = nullptr; - // We need to transition in and out of kSuspended so we suspend, gain the lock, wait, unsuspend - // and lose lock, gain lock again, make sure another hasn't happened, then continue. - MutexLock mu(self, lock_); - WaitForPotentialCollectionToCompleteRunnable(self); { - ScopedCodeCacheWrite scc(this); - memory = AllocateCode(total_size); - if (memory == nullptr) { - return nullptr; - } - code_ptr = memory + header_size; - - std::copy(code, code + code_size, code_ptr); - method_header = OatQuickMethodHeader::FromCodePointer(code_ptr); - new (method_header) OatQuickMethodHeader( - (stack_map != nullptr) ? code_ptr - stack_map : 0u, - code_size); - // Flush caches before we remove write permission because some ARMv8 Qualcomm kernels may - // trigger a segfault if a page fault occurs when requesting a cache maintenance operation. - // This is a kernel bug that we need to work around until affected devices (e.g. Nexus 5X and - // 6P) stop being supported or their kernels are fixed. - // - // For reference, this behavior is caused by this commit: - // https://android.googlesource.com/kernel/msm/+/3fbe6bc28a6b9939d0650f2f17eb5216c719950c - FlushInstructionCache(reinterpret_cast<char*>(code_ptr), - reinterpret_cast<char*>(code_ptr + code_size)); - DCHECK(!Runtime::Current()->IsAotCompiler()); - if (has_should_deoptimize_flag) { - method_header->SetHasShouldDeoptimizeFlag(); + ScopedThreadSuspension sts(self, kSuspended); + MutexLock mu(self, lock_); + WaitForPotentialCollectionToComplete(self); + { + ScopedCodeCacheWrite scc(this); + memory = AllocateCode(total_size); + if (memory == nullptr) { + return nullptr; + } + code_ptr = memory + header_size; + + std::copy(code, code + code_size, code_ptr); + method_header = OatQuickMethodHeader::FromCodePointer(code_ptr); + new (method_header) OatQuickMethodHeader( + (stack_map != nullptr) ? code_ptr - stack_map : 0u, + code_size); + // Flush caches before we remove write permission because some ARMv8 Qualcomm kernels may + // trigger a segfault if a page fault occurs when requesting a cache maintenance operation. + // This is a kernel bug that we need to work around until affected devices (e.g. Nexus 5X and + // 6P) stop being supported or their kernels are fixed. + // + // For reference, this behavior is caused by this commit: + // https://android.googlesource.com/kernel/msm/+/3fbe6bc28a6b9939d0650f2f17eb5216c719950c + FlushInstructionCache(reinterpret_cast<char*>(code_ptr), + reinterpret_cast<char*>(code_ptr + code_size)); + DCHECK(!Runtime::Current()->IsAotCompiler()); + if (has_should_deoptimize_flag) { + method_header->SetHasShouldDeoptimizeFlag(); + } } number_of_compilations_++; } // We need to update the entry point in the runnable state for the instrumentation. { - // The following needs to be guarded by cha_lock_ also. Otherwise it's possible that the - // compiled code is considered invalidated by some class linking, but below we still make the - // compiled code valid for the method. Need cha_lock_ for checking all single-implementation - // flags and register dependencies. + // Need cha_lock_ for checking all single-implementation flags and register + // dependencies. MutexLock cha_mu(self, *Locks::cha_lock_); bool single_impl_still_valid = true; for (ArtMethod* single_impl : cha_single_implementation_list) { @@ -848,6 +826,16 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, single_impl, method, method_header); } + if (!method->IsNative()) { + // We need to do this before grabbing the lock_ because it needs to be able to see the string + // InternTable. Native methods do not have roots. + DCheckRootsAreValid(roots); + } + + // The following needs to be guarded by cha_lock_ also. Otherwise it's + // possible that the compiled code is considered invalidated by some class linking, + // but below we still make the compiled code valid for the method. + MutexLock mu(self, lock_); if (UNLIKELY(method->IsNative())) { auto it = jni_stubs_map_.find(JniStubKey(method)); DCHECK(it != jni_stubs_map_.end()) @@ -879,6 +867,11 @@ uint8_t* JitCodeCache::CommitCodeInternal(Thread* self, method, method_header->GetEntryPoint()); } } + if (collection_in_progress_) { + // We need to update the live bitmap if there is a GC to ensure it sees this new + // code. + GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(code_ptr)); + } VLOG(jit) << "JIT added (osr=" << std::boolalpha << osr << std::noboolalpha << ") " << ArtMethod::PrettyMethod(method) << "@" << method diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h index 6cdf5dccad..29f9c9cf43 100644 --- a/runtime/jit/jit_code_cache.h +++ b/runtime/jit/jit_code_cache.h @@ -314,12 +314,6 @@ class JitCodeCache { REQUIRES(lock_) REQUIRES_SHARED(Locks::mutator_lock_); - // If a collection is in progress, wait for it to finish. Return whether the thread actually - // waited. Must be called with the mutator lock. The non-mutator lock version should be used if - // possible. This will release then re-acquire the mutator lock. - bool WaitForPotentialCollectionToCompleteRunnable(Thread* self) - REQUIRES(lock_, !Roles::uninterruptible_) REQUIRES_SHARED(Locks::mutator_lock_); - // If a collection is in progress, wait for it to finish. Return // whether the thread actually waited. bool WaitForPotentialCollectionToComplete(Thread* self) |