OSDN Git Service

ART: Add generic system-weak holder infrastructure
authorAndreas Gampe <agampe@google.com>
Fri, 9 Sep 2016 03:29:18 +0000 (20:29 -0700)
committerAndreas Gampe <agampe@google.com>
Mon, 12 Sep 2016 19:52:22 +0000 (12:52 -0700)
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

build/Android.gtest.mk
runtime/gc/collector_type.h
runtime/gc/gc_cause.h
runtime/gc/system_weak.h [new file with mode: 0644]
runtime/gc/system_weak_test.cc [new file with mode: 0644]
runtime/runtime.cc
runtime/runtime.h

index c09241a..7ad37ac 100644 (file)
@@ -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 \
index b342cc7..7014357 100644 (file)
@@ -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);
 
index 4348a41..f54f0e4 100644 (file)
@@ -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 (file)
index 0000000..3910a28
--- /dev/null
@@ -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 (file)
index 0000000..7c1ec8a
--- /dev/null
@@ -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 <stdint.h>
+#include <stdio.h>
+#include <memory>
+
+#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<kWithoutReadBarrier>();
+    mirror::Object* new_object = old_object == nullptr ? nullptr : visitor->IsMarked(old_object);
+    weak_ = GcRoot<mirror::Object>(new_object);
+
+    sweep_count_++;
+  }
+
+  GcRoot<mirror::Object> 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<mirror::Object> 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<mirror::Object> 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<mirror::String> s(hs.NewHandle(mirror::String::AllocFromModifiedUtf8(soa.Self(), "ABC")));
+  cswh.Set(GcRoot<mirror::Object>(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::Object>(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<mirror::String> s(hs.NewHandle(mirror::String::AllocFromModifiedUtf8(soa.Self(), "ABC")));
+  cswh.Set(GcRoot<mirror::Object>(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
index a365a73..71fc3ba 100644 (file)
 #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
index 44f765a..38601fd 100644 (file)
@@ -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<gc::AbstractSystemWeakHolder*> system_weak_holders_;
+
   DISALLOW_COPY_AND_ASSIGN(Runtime);
 };
 std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs);