From: Andreas Gampe Date: Fri, 9 Sep 2016 03:29:18 +0000 (-0700) Subject: ART: Add generic system-weak holder infrastructure X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=fda5714edb368270b7ef639054f4cba1d5f4874c;p=android-x86%2Fart.git ART: Add generic system-weak holder infrastructure Add an "interface" for a generic system-weak holder that is integrated with the well-known instances in Runtime. Add a simple implementation handling synchronization. Add a test. Bug: 31385027 Test: m test-art-host-gtest-system_weak_test Test: m ART_USE_READ_BARRIER=true test-art-host-gtest-system_weak_test Test: m ART_DEFAULT_GC_TYPE=SS test-art-host-gtest-system_weak_test Test: m ART_DEFAULT_GC_TYPE=GSS test-art-host-gtest-system_weak_test Change-Id: I1100e2cbd9ee57860993d0039de73d197681c542 --- diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index c09241a38..7ad37ac97 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -237,6 +237,7 @@ RUNTIME_GTEST_COMMON_SRC_FILES := \ runtime/gc/space/rosalloc_space_static_test.cc \ runtime/gc/space/rosalloc_space_random_test.cc \ runtime/gc/space/space_create_test.cc \ + runtime/gc/system_weak_test.cc \ runtime/gc/task_processor_test.cc \ runtime/gtest_test.cc \ runtime/handle_scope_test.cc \ diff --git a/runtime/gc/collector_type.h b/runtime/gc/collector_type.h index b342cc7aa..701435752 100644 --- a/runtime/gc/collector_type.h +++ b/runtime/gc/collector_type.h @@ -55,6 +55,8 @@ enum CollectorType { kCollectorTypeClassLinker, // JIT Code cache fake collector. kCollectorTypeJitCodeCache, + // Fake collector for installing/removing a system-weak holder. + kCollectorTypeAddRemoveSystemWeakHolder, }; std::ostream& operator<<(std::ostream& os, const CollectorType& collector_type); diff --git a/runtime/gc/gc_cause.h b/runtime/gc/gc_cause.h index 4348a411a..f54f0e490 100644 --- a/runtime/gc/gc_cause.h +++ b/runtime/gc/gc_cause.h @@ -51,6 +51,8 @@ enum GcCause { kGcCauseClassLinker, // Not a real GC cause, used to implement exclusion between code cache metadata and GC. kGcCauseJitCodeCache, + // Not a real GC cause, used to add or remove system-weak holders. + kGcCauseAddRemoveSystemWeakHolder, }; const char* PrettyCause(GcCause cause); diff --git a/runtime/gc/system_weak.h b/runtime/gc/system_weak.h new file mode 100644 index 000000000..3910a280c --- /dev/null +++ b/runtime/gc/system_weak.h @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_RUNTIME_GC_SYSTEM_WEAK_H_ +#define ART_RUNTIME_GC_SYSTEM_WEAK_H_ + +#include "base/mutex.h" +#include "object_callbacks.h" +#include "thread-inl.h" + +namespace art { +namespace gc { + +class AbstractSystemWeakHolder { + public: + virtual ~AbstractSystemWeakHolder() {} + + virtual void Allow() REQUIRES_SHARED(Locks::mutator_lock_) = 0; + virtual void Disallow() REQUIRES_SHARED(Locks::mutator_lock_) = 0; + virtual void Broadcast() REQUIRES_SHARED(Locks::mutator_lock_) = 0; + + virtual void Sweep(IsMarkedVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_) = 0; +}; + +class SystemWeakHolder : public AbstractSystemWeakHolder { + public: + explicit SystemWeakHolder(LockLevel level) + : allow_disallow_lock_("SystemWeakHolder", level), + new_weak_condition_("SystemWeakHolder new condition", allow_disallow_lock_), + allow_new_system_weak_(true) { + } + virtual ~SystemWeakHolder() {} + + void Allow() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + CHECK(!kUseReadBarrier); + MutexLock mu(Thread::Current(), allow_disallow_lock_); + allow_new_system_weak_ = true; + new_weak_condition_.Broadcast(Thread::Current()); + } + + void Disallow() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + CHECK(!kUseReadBarrier); + MutexLock mu(Thread::Current(), allow_disallow_lock_); + allow_new_system_weak_ = false; + } + + void Broadcast() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + CHECK(kUseReadBarrier); + MutexLock mu(Thread::Current(), allow_disallow_lock_); + new_weak_condition_.Broadcast(Thread::Current()); + } + + protected: + void Wait(Thread* self) REQUIRES_SHARED(allow_disallow_lock_) { + // Wait for GC's sweeping to complete and allow new records + while (UNLIKELY((!kUseReadBarrier && !allow_new_system_weak_) || + (kUseReadBarrier && !self->GetWeakRefAccessEnabled()))) { + new_weak_condition_.WaitHoldingLocks(self); + } + } + + Mutex allow_disallow_lock_; + ConditionVariable new_weak_condition_ GUARDED_BY(allow_disallow_lock_); + bool allow_new_system_weak_ GUARDED_BY(allow_disallow_lock_); +}; + +} // namespace gc +} // namespace art + +#endif // ART_RUNTIME_GC_SYSTEM_WEAK_H_ diff --git a/runtime/gc/system_weak_test.cc b/runtime/gc/system_weak_test.cc new file mode 100644 index 000000000..7c1ec8af4 --- /dev/null +++ b/runtime/gc/system_weak_test.cc @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "system_weak.h" + +#include +#include +#include + +#include "base/mutex.h" +#include "collector_type.h" +#include "common_runtime_test.h" +#include "handle_scope-inl.h" +#include "heap.h" +#include "mirror/string.h" +#include "scoped_thread_state_change.h" +#include "thread_list.h" + +namespace art { +namespace gc { + +class SystemWeakTest : public CommonRuntimeTest { +}; + +struct CountingSystemWeakHolder : public SystemWeakHolder { + CountingSystemWeakHolder() + : SystemWeakHolder(kAllocTrackerLock), + allow_count_(0), + disallow_count_(0), + sweep_count_(0) {} + + void Allow() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + SystemWeakHolder::Allow(); + + allow_count_++; + } + + void Disallow() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + SystemWeakHolder::Disallow(); + + disallow_count_++; + } + + void Broadcast() OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + SystemWeakHolder::Broadcast(); + + allow_count_++; + } + + void Sweep(IsMarkedVisitor* visitor) OVERRIDE + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + MutexLock mu(Thread::Current(), allow_disallow_lock_); + mirror::Object* old_object = weak_.Read(); + mirror::Object* new_object = old_object == nullptr ? nullptr : visitor->IsMarked(old_object); + weak_ = GcRoot(new_object); + + sweep_count_++; + } + + GcRoot Get() + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + Thread* self = Thread::Current(); + MutexLock mu(self, allow_disallow_lock_); + Wait(self); + + return weak_; + } + + void Set(GcRoot obj) + REQUIRES_SHARED(Locks::mutator_lock_) + REQUIRES(!allow_disallow_lock_) { + Thread* self = Thread::Current(); + MutexLock mu(self, allow_disallow_lock_); + Wait(self); + + weak_ = obj; + } + + size_t allow_count_; + size_t disallow_count_; + size_t sweep_count_; + GcRoot weak_ GUARDED_BY(allow_disallow_lock_); +}; + +static bool CollectorDoesAllowOrBroadcast() { + CollectorType type = Runtime::Current()->GetHeap()->CurrentCollectorType(); + switch (type) { + case CollectorType::kCollectorTypeCMS: + case CollectorType::kCollectorTypeCC: + return true; + + default: + return false; + } +} + +static bool CollectorDoesDisallow() { + CollectorType type = Runtime::Current()->GetHeap()->CurrentCollectorType(); + switch (type) { + case CollectorType::kCollectorTypeCMS: + return true; + + default: + return false; + } +} + +TEST_F(SystemWeakTest, Keep) { + CountingSystemWeakHolder cswh; + Runtime::Current()->AddSystemWeakHolder(&cswh); + + ScopedObjectAccess soa(Thread::Current()); + + StackHandleScope<1> hs(soa.Self()); + + // We use Strings because they are very easy to allocate. + Handle s(hs.NewHandle(mirror::String::AllocFromModifiedUtf8(soa.Self(), "ABC"))); + cswh.Set(GcRoot(s.Get())); + + // Trigger a GC. + Runtime::Current()->GetHeap()->CollectGarbage(false); + + // Expect the holder to have been called. + EXPECT_EQ(CollectorDoesAllowOrBroadcast() ? 1U : 0U, cswh.allow_count_); + EXPECT_EQ(CollectorDoesDisallow() ? 1U : 0U, cswh.disallow_count_); + EXPECT_EQ(1U, cswh.sweep_count_); + + // Expect the weak to not be cleared. + EXPECT_FALSE(cswh.Get().IsNull()); + EXPECT_EQ(cswh.Get().Read(), s.Get()); +} + +TEST_F(SystemWeakTest, Discard) { + CountingSystemWeakHolder cswh; + Runtime::Current()->AddSystemWeakHolder(&cswh); + + ScopedObjectAccess soa(Thread::Current()); + + cswh.Set(GcRoot(mirror::String::AllocFromModifiedUtf8(soa.Self(), "ABC"))); + + // Trigger a GC. + Runtime::Current()->GetHeap()->CollectGarbage(false); + + // Expect the holder to have been called. + EXPECT_EQ(CollectorDoesAllowOrBroadcast() ? 1U : 0U, cswh.allow_count_); + EXPECT_EQ(CollectorDoesDisallow() ? 1U : 0U, cswh.disallow_count_); + EXPECT_EQ(1U, cswh.sweep_count_); + + // Expect the weak to be cleared. + EXPECT_TRUE(cswh.Get().IsNull()); +} + +TEST_F(SystemWeakTest, Remove) { + CountingSystemWeakHolder cswh; + Runtime::Current()->AddSystemWeakHolder(&cswh); + + ScopedObjectAccess soa(Thread::Current()); + + StackHandleScope<1> hs(soa.Self()); + + // We use Strings because they are very easy to allocate. + Handle s(hs.NewHandle(mirror::String::AllocFromModifiedUtf8(soa.Self(), "ABC"))); + cswh.Set(GcRoot(s.Get())); + + // Trigger a GC. + Runtime::Current()->GetHeap()->CollectGarbage(false); + + // Expect the holder to have been called. + ASSERT_EQ(CollectorDoesAllowOrBroadcast() ? 1U : 0U, cswh.allow_count_); + ASSERT_EQ(CollectorDoesDisallow() ? 1U : 0U, cswh.disallow_count_); + ASSERT_EQ(1U, cswh.sweep_count_); + + // Expect the weak to not be cleared. + ASSERT_FALSE(cswh.Get().IsNull()); + ASSERT_EQ(cswh.Get().Read(), s.Get()); + + // Remove the holder. + Runtime::Current()->RemoveSystemWeakHolder(&cswh); + + // Trigger another GC. + Runtime::Current()->GetHeap()->CollectGarbage(false); + + // Expectation: no change in the numbers. + EXPECT_EQ(CollectorDoesAllowOrBroadcast() ? 1U : 0U, cswh.allow_count_); + EXPECT_EQ(CollectorDoesDisallow() ? 1U : 0U, cswh.disallow_count_); + EXPECT_EQ(1U, cswh.sweep_count_); +} + +} // namespace gc +} // namespace art diff --git a/runtime/runtime.cc b/runtime/runtime.cc index a365a737d..71fc3ba6c 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -71,8 +71,10 @@ #include "fault_handler.h" #include "gc/accounting/card_table-inl.h" #include "gc/heap.h" +#include "gc/scoped_gc_critical_section.h" #include "gc/space/image_space.h" #include "gc/space/space-inl.h" +#include "gc/system_weak.h" #include "handle_scope-inl.h" #include "image-inl.h" #include "instrumentation.h" @@ -473,6 +475,11 @@ void Runtime::SweepSystemWeaks(IsMarkedVisitor* visitor) { GetMonitorList()->SweepMonitorList(visitor); GetJavaVM()->SweepJniWeakGlobals(visitor); GetHeap()->SweepAllocationRecords(visitor); + + // All other generic system-weak holders. + for (gc::AbstractSystemWeakHolder* holder : system_weak_holders_) { + holder->Sweep(visitor); + } } bool Runtime::ParseOptions(const RuntimeOptions& raw_options, @@ -1708,6 +1715,11 @@ void Runtime::DisallowNewSystemWeaks() { intern_table_->ChangeWeakRootState(gc::kWeakRootStateNoReadsOrWrites); java_vm_->DisallowNewWeakGlobals(); heap_->DisallowNewAllocationRecords(); + + // All other generic system-weak holders. + for (gc::AbstractSystemWeakHolder* holder : system_weak_holders_) { + holder->Disallow(); + } } void Runtime::AllowNewSystemWeaks() { @@ -1716,6 +1728,11 @@ void Runtime::AllowNewSystemWeaks() { intern_table_->ChangeWeakRootState(gc::kWeakRootStateNormal); // TODO: Do this in the sweeping. java_vm_->AllowNewWeakGlobals(); heap_->AllowNewAllocationRecords(); + + // All other generic system-weak holders. + for (gc::AbstractSystemWeakHolder* holder : system_weak_holders_) { + holder->Allow(); + } } void Runtime::BroadcastForNewSystemWeaks() { @@ -1726,6 +1743,11 @@ void Runtime::BroadcastForNewSystemWeaks() { intern_table_->BroadcastForNewInterns(); java_vm_->BroadcastForNewWeakGlobals(); heap_->BroadcastForNewAllocationRecords(); + + // All other generic system-weak holders. + for (gc::AbstractSystemWeakHolder* holder : system_weak_holders_) { + holder->Broadcast(); + } } void Runtime::SetInstructionSet(InstructionSet instruction_set) { @@ -2064,4 +2086,21 @@ char** Runtime::EnvSnapshot::GetSnapshot() const { return c_env_vector_.get(); } +void Runtime::AddSystemWeakHolder(gc::AbstractSystemWeakHolder* holder) { + gc::ScopedGCCriticalSection gcs(Thread::Current(), + gc::kGcCauseAddRemoveSystemWeakHolder, + gc::kCollectorTypeAddRemoveSystemWeakHolder); + system_weak_holders_.push_back(holder); +} + +void Runtime::RemoveSystemWeakHolder(gc::AbstractSystemWeakHolder* holder) { + gc::ScopedGCCriticalSection gcs(Thread::Current(), + gc::kGcCauseAddRemoveSystemWeakHolder, + gc::kCollectorTypeAddRemoveSystemWeakHolder); + auto it = std::find(system_weak_holders_.begin(), system_weak_holders_.end(), holder); + if (it != system_weak_holders_.end()) { + system_weak_holders_.erase(it); + } +} + } // namespace art diff --git a/runtime/runtime.h b/runtime/runtime.h index 44f765ac3..38601fdf7 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -43,6 +43,7 @@ namespace art { namespace gc { + class AbstractSystemWeakHolder; class Heap; namespace collector { class GarbageCollector; @@ -648,6 +649,9 @@ class Runtime { return env_snapshot_.GetSnapshot(); } + void AddSystemWeakHolder(gc::AbstractSystemWeakHolder* holder); + void RemoveSystemWeakHolder(gc::AbstractSystemWeakHolder* holder); + private: static void InitPlatformSignalHandlers(); @@ -884,6 +888,9 @@ class Runtime { DISALLOW_COPY_AND_ASSIGN(EnvSnapshot); } env_snapshot_; + // Generic system-weak holders. + std::vector system_weak_holders_; + DISALLOW_COPY_AND_ASSIGN(Runtime); }; std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs);