From 624468cd401cc1ac0dd70c746301e0788a597759 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 31 Mar 2014 15:14:47 -0700 Subject: [PATCH] Make the support code for read barriers a bit more general. Add an option for Baker in addition to Brooks. Bug: 12687968 Change-Id: I8a31db817ff6686c72951b6534f588228e270b11 --- compiler/image_writer.cc | 12 ++++++----- runtime/asm_support.h | 4 ++-- runtime/class_linker.cc | 8 +++---- runtime/class_linker_test.cc | 14 ++++++------ runtime/gc/collector/mark_sweep.cc | 12 +++++------ runtime/gc/collector/semi_space-inl.h | 4 ++-- runtime/gc/collector/semi_space.cc | 12 ++++++----- runtime/gc/heap-inl.h | 8 ++++--- runtime/gc/heap.cc | 12 ++++++----- runtime/gc/space/image_space.cc | 6 ++---- runtime/gc/space/space_test.h | 4 ++-- runtime/globals.h | 16 ++++++++++---- runtime/mirror/object-inl.h | 32 +++++++++++++++++----------- runtime/mirror/object.h | 14 ++++++------ runtime/{brooks_pointer.h => read_barrier.h} | 22 +++++++++++++------ 15 files changed, 106 insertions(+), 74 deletions(-) rename runtime/{brooks_pointer.h => read_barrier.h} (56%) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 682418310..040519835 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -610,11 +610,13 @@ class FixupVisitor { void ImageWriter::FixupObject(Object* orig, Object* copy) { DCHECK(orig != nullptr); DCHECK(copy != nullptr); - if (kUseBrooksPointer) { - orig->AssertSelfBrooksPointer(); - // Note the address 'copy' isn't the same as the image address of 'orig'. - copy->SetBrooksPointer(GetImageAddress(orig)); - DCHECK_EQ(copy->GetBrooksPointer(), GetImageAddress(orig)); + if (kUseBakerOrBrooksReadBarrier) { + orig->AssertReadBarrierPointer(); + if (kUseBrooksReadBarrier) { + // Note the address 'copy' isn't the same as the image address of 'orig'. + copy->SetReadBarrierPointer(GetImageAddress(orig)); + DCHECK_EQ(copy->GetReadBarrierPointer(), GetImageAddress(orig)); + } } FixupVisitor visitor(this, copy); orig->VisitReferences(visitor, visitor); diff --git a/runtime/asm_support.h b/runtime/asm_support.h index 0c1a72aac..8ef407d0a 100644 --- a/runtime/asm_support.h +++ b/runtime/asm_support.h @@ -17,7 +17,7 @@ #ifndef ART_RUNTIME_ASM_SUPPORT_H_ #define ART_RUNTIME_ASM_SUPPORT_H_ -#include "brooks_pointer.h" +#include "read_barrier.h" // Value loaded into rSUSPEND for quick. When this value is counted down to zero we do a suspend // check. @@ -27,7 +27,7 @@ #define CLASS_OFFSET 0 #define LOCK_WORD_OFFSET 4 -#ifndef USE_BROOKS_POINTER +#ifndef USE_BAKER_OR_BROOKS_READ_BARRIER // Offsets within java.lang.Class. #define CLASS_COMPONENT_TYPE_OFFSET 12 diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 08ea12386..395749394 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -206,8 +206,8 @@ void ClassLinker::InitFromCompiler(const std::vector& boot_class CHECK(java_lang_Class.get() != NULL); mirror::Class::SetClassClass(java_lang_Class.get()); java_lang_Class->SetClass(java_lang_Class.get()); - if (kUseBrooksPointer) { - java_lang_Class->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + java_lang_Class->AssertReadBarrierPointer(); } java_lang_Class->SetClassSize(sizeof(mirror::ClassClass)); heap->DecrementDisableMovingGC(self); @@ -1864,8 +1864,8 @@ void ClassLinker::LoadClass(const DexFile& dex_file, CHECK(descriptor != NULL); klass->SetClass(GetClassRoot(kJavaLangClass)); - if (kUseBrooksPointer) { - klass->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + klass->AssertReadBarrierPointer(); } uint32_t access_flags = dex_class_def.access_flags_; // Make sure that none of our runtime-only flags are set. diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc index 7eb7b01e6..5b72a4496 100644 --- a/runtime/class_linker_test.cc +++ b/runtime/class_linker_test.cc @@ -452,9 +452,9 @@ struct ObjectOffsets : public CheckOffsets { // alphabetical 32-bit offsets.push_back(CheckOffset(OFFSETOF_MEMBER(mirror::Object, monitor_), "shadow$_monitor_")); -#ifdef USE_BROOKS_POINTER - offsets.push_back(CheckOffset(OFFSETOF_MEMBER(mirror::Object, x_brooks_ptr_), "shadow$_x_brooks_ptr_")); - offsets.push_back(CheckOffset(OFFSETOF_MEMBER(mirror::Object, x_padding_), "shadow$_x_padding_")); +#ifdef USE_BAKER_OR_BROOKS_READ_BARRIER + offsets.push_back(CheckOffset(OFFSETOF_MEMBER(mirror::Object, x_rb_ptr_), "shadow$_x_rb_ptr_")); + offsets.push_back(CheckOffset(OFFSETOF_MEMBER(mirror::Object, x_xpadding_), "shadow$_x_xpadding_")); #endif }; }; @@ -731,7 +731,7 @@ TEST_F(ClassLinkerTest, FindClass) { EXPECT_FALSE(JavaLangObject->IsSynthetic()); EXPECT_EQ(2U, JavaLangObject->NumDirectMethods()); EXPECT_EQ(11U, JavaLangObject->NumVirtualMethods()); - if (!kUseBrooksPointer) { + if (!kUseBakerOrBrooksReadBarrier) { EXPECT_EQ(2U, JavaLangObject->NumInstanceFields()); } else { EXPECT_EQ(4U, JavaLangObject->NumInstanceFields()); @@ -740,11 +740,11 @@ TEST_F(ClassLinkerTest, FindClass) { EXPECT_STREQ(fh.GetName(), "shadow$_klass_"); fh.ChangeField(JavaLangObject->GetInstanceField(1)); EXPECT_STREQ(fh.GetName(), "shadow$_monitor_"); - if (kUseBrooksPointer) { + if (kUseBakerOrBrooksReadBarrier) { fh.ChangeField(JavaLangObject->GetInstanceField(2)); - EXPECT_STREQ(fh.GetName(), "shadow$_x_brooks_ptr_"); + EXPECT_STREQ(fh.GetName(), "shadow$_x_rb_ptr_"); fh.ChangeField(JavaLangObject->GetInstanceField(3)); - EXPECT_STREQ(fh.GetName(), "shadow$_x_padding_"); + EXPECT_STREQ(fh.GetName(), "shadow$_x_xpadding_"); } EXPECT_EQ(0U, JavaLangObject->NumStaticFields()); diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc index 91ccd647d..ca2d0bd6d 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -343,9 +343,9 @@ void MarkSweep::MarkHeapReferenceCallback(mirror::HeapReference* inline void MarkSweep::MarkObjectNonNull(Object* obj) { DCHECK(obj != nullptr); - if (kUseBrooksPointer) { - // Verify all the objects have the correct Brooks pointer installed. - obj->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + // Verify all the objects have the correct pointer installed. + obj->AssertReadBarrierPointer(); } if (immune_region_.ContainsObject(obj)) { if (kCountMarkedObjects) { @@ -415,9 +415,9 @@ bool MarkSweep::MarkLargeObject(const Object* obj, bool set) { inline bool MarkSweep::MarkObjectParallel(const Object* obj) { DCHECK(obj != nullptr); - if (kUseBrooksPointer) { - // Verify all the objects have the correct Brooks pointer installed. - obj->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + // Verify all the objects have the correct pointer installed. + obj->AssertReadBarrierPointer(); } if (immune_region_.ContainsObject(obj)) { DCHECK(IsMarked(obj)); diff --git a/runtime/gc/collector/semi_space-inl.h b/runtime/gc/collector/semi_space-inl.h index d60298b68..df731ff0c 100644 --- a/runtime/gc/collector/semi_space-inl.h +++ b/runtime/gc/collector/semi_space-inl.h @@ -45,9 +45,9 @@ inline void SemiSpace::MarkObject( if (obj == nullptr) { return; } - if (kUseBrooksPointer) { + if (kUseBakerOrBrooksReadBarrier) { // Verify all the objects have the correct forward pointer installed. - obj->AssertSelfBrooksPointer(); + obj->AssertReadBarrierPointer(); } if (!immune_region_.ContainsObject(obj)) { if (from_space_->HasAddress(obj)) { diff --git a/runtime/gc/collector/semi_space.cc b/runtime/gc/collector/semi_space.cc index 222bd6333..1366858fe 100644 --- a/runtime/gc/collector/semi_space.cc +++ b/runtime/gc/collector/semi_space.cc @@ -561,11 +561,13 @@ mirror::Object* SemiSpace::MarkNonForwardedObject(mirror::Object* obj) { // references. saved_bytes_ += CopyAvoidingDirtyingPages(reinterpret_cast(forward_address), obj, object_size); - if (kUseBrooksPointer) { - obj->AssertSelfBrooksPointer(); - DCHECK_EQ(forward_address->GetBrooksPointer(), obj); - forward_address->SetBrooksPointer(forward_address); - forward_address->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + obj->AssertReadBarrierPointer(); + if (kUseBrooksReadBarrier) { + DCHECK_EQ(forward_address->GetReadBarrierPointer(), obj); + forward_address->SetReadBarrierPointer(forward_address); + } + forward_address->AssertReadBarrierPointer(); } if (to_space_live_bitmap_ != nullptr) { to_space_live_bitmap_->Set(forward_address); diff --git a/runtime/gc/heap-inl.h b/runtime/gc/heap-inl.h index 8bfe79306..25f20d621 100644 --- a/runtime/gc/heap-inl.h +++ b/runtime/gc/heap-inl.h @@ -73,9 +73,11 @@ inline mirror::Object* Heap::AllocObjectWithAllocator(Thread* self, mirror::Clas DCHECK_GT(bytes_allocated, 0u); DCHECK_GT(usable_size, 0u); obj->SetClass(klass); - if (kUseBrooksPointer) { - obj->SetBrooksPointer(obj); - obj->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + if (kUseBrooksReadBarrier) { + obj->SetReadBarrierPointer(obj); + } + obj->AssertReadBarrierPointer(); } if (collector::SemiSpace::kUseRememberedSet && UNLIKELY(allocator == kAllocatorTypeNonMoving)) { // (Note this if statement will be constant folded away for the diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index 1a32a9a05..bc506688b 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -1605,11 +1605,13 @@ class ZygoteCompactingCollector FINAL : public collector::SemiSpace { } // Copy the object over to its new location. memcpy(reinterpret_cast(forward_address), obj, object_size); - if (kUseBrooksPointer) { - obj->AssertSelfBrooksPointer(); - DCHECK_EQ(forward_address->GetBrooksPointer(), obj); - forward_address->SetBrooksPointer(forward_address); - forward_address->AssertSelfBrooksPointer(); + if (kUseBakerOrBrooksReadBarrier) { + obj->AssertReadBarrierPointer(); + if (kUseBrooksReadBarrier) { + DCHECK_EQ(forward_address->GetReadBarrierPointer(), obj); + forward_address->SetReadBarrierPointer(forward_address); + } + forward_address->AssertReadBarrierPointer(); } return forward_address; } diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc index bb52c6690..9a2815a1d 100644 --- a/runtime/gc/space/image_space.cc +++ b/runtime/gc/space/image_space.cc @@ -166,10 +166,8 @@ void ImageSpace::VerifyImageAllocations() { mirror::Object* obj = reinterpret_cast(current); CHECK(live_bitmap_->Test(obj)); CHECK(obj->GetClass() != nullptr) << "Image object at address " << obj << " has null class"; - if (kUseBrooksPointer) { - CHECK(obj->GetBrooksPointer() == obj) - << "Bad Brooks pointer: obj=" << reinterpret_cast(obj) - << " brooks_ptr=" << reinterpret_cast(obj->GetBrooksPointer()); + if (kUseBakerOrBrooksReadBarrier) { + obj->AssertReadBarrierPointer(); } current += RoundUp(obj->SizeOf(), kObjectAlignment); } diff --git a/runtime/gc/space/space_test.h b/runtime/gc/space/space_test.h index 6d3602c47..5c735df45 100644 --- a/runtime/gc/space/space_test.h +++ b/runtime/gc/space/space_test.h @@ -85,8 +85,8 @@ class SpaceTest : public CommonRuntimeTest { EXPECT_GE(size, SizeOfZeroLengthByteArray()); EXPECT_TRUE(byte_array_class != nullptr); o->SetClass(byte_array_class); - if (kUseBrooksPointer) { - o->SetBrooksPointer(o); + if (kUseBrooksReadBarrier) { + o->SetReadBarrierPointer(o); } mirror::Array* arr = o->AsArray(); size_t header_size = SizeOfZeroLengthByteArray(); diff --git a/runtime/globals.h b/runtime/globals.h index 9c6fa0dec..f2d686254 100644 --- a/runtime/globals.h +++ b/runtime/globals.h @@ -19,7 +19,7 @@ #include #include -#include "brooks_pointer.h" +#include "read_barrier.h" namespace art { @@ -97,12 +97,20 @@ static constexpr bool kMovingMethods = false; // code, if possible. static constexpr bool kEmbedClassInCode = true; -#ifdef USE_BROOKS_POINTER -static constexpr bool kUseBrooksPointer = true; +#ifdef USE_BAKER_READ_BARRIER +static constexpr bool kUseBakerReadBarrier = true; #else -static constexpr bool kUseBrooksPointer = false; +static constexpr bool kUseBakerReadBarrier = false; #endif +#ifdef USE_BROOKS_READ_BARRIER +static constexpr bool kUseBrooksReadBarrier = true; +#else +static constexpr bool kUseBrooksReadBarrier = false; +#endif + +static constexpr bool kUseBakerOrBrooksReadBarrier = kUseBakerReadBarrier || kUseBrooksReadBarrier; + // If true, references within the heap are poisoned (negated). static constexpr bool kPoisonHeapReferences = false; diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h index 527b8a65e..b6c140d6d 100644 --- a/runtime/mirror/object-inl.h +++ b/runtime/mirror/object-inl.h @@ -93,33 +93,41 @@ inline void Object::Wait(Thread* self, int64_t ms, int32_t ns) { Monitor::Wait(self, this, ms, ns, true, kTimedWaiting); } -inline Object* Object::GetBrooksPointer() { -#ifdef USE_BROOKS_POINTER - DCHECK(kUseBrooksPointer); - return GetFieldObject(OFFSET_OF_OBJECT_MEMBER(Object, x_brooks_ptr_), false); +inline Object* Object::GetReadBarrierPointer() { +#ifdef USE_BAKER_OR_BROOKS_READ_BARRIER + DCHECK(kUseBakerOrBrooksReadBarrier); + return GetFieldObject(OFFSET_OF_OBJECT_MEMBER(Object, x_rb_ptr_), false); #else LOG(FATAL) << "Unreachable"; return nullptr; #endif } -inline void Object::SetBrooksPointer(Object* brooks_pointer) { -#ifdef USE_BROOKS_POINTER - DCHECK(kUseBrooksPointer); +inline void Object::SetReadBarrierPointer(Object* rb_pointer) { +#ifdef USE_BAKER_OR_BROOKS_READ_BARRIER + DCHECK(kUseBakerOrBrooksReadBarrier); // We don't mark the card as this occurs as part of object allocation. Not all objects have // backing cards, such as large objects. SetFieldObjectWithoutWriteBarrier( - OFFSET_OF_OBJECT_MEMBER(Object, x_brooks_ptr_), brooks_pointer, false); + OFFSET_OF_OBJECT_MEMBER(Object, x_rb_ptr_), rb_pointer, false); #else LOG(FATAL) << "Unreachable"; #endif } -inline void Object::AssertSelfBrooksPointer() const { -#ifdef USE_BROOKS_POINTER - DCHECK(kUseBrooksPointer); +inline void Object::AssertReadBarrierPointer() const { +#if defined(USE_BAKER_READ_BARRIER) + DCHECK(kUseBakerReadBarrier); Object* obj = const_cast(this); - DCHECK_EQ(obj, obj->GetBrooksPointer()); + DCHECK(obj->GetReadBarrierPointer() == nullptr) + << "Bad Baker pointer: obj=" << reinterpret_cast(obj) + << " ptr=" << reinterpret_cast(obj->GetReadBarrierPointer()); +#elif defined(USE_BROOKS_READ_BARRIER) + DCHECK(kUseBrooksReadBarrier); + Object* obj = const_cast(this); + DCHECK_EQ(obj, obj->GetReadBarrierPointer()) + << "Bad Brooks pointer: obj=" << reinterpret_cast(obj) + << " ptr=" << reinterpret_cast(obj->GetReadBarrierPointer()); #else LOG(FATAL) << "Unreachable"; #endif diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h index 0a778289a..1ac23ce6c 100644 --- a/runtime/mirror/object.h +++ b/runtime/mirror/object.h @@ -78,9 +78,9 @@ class MANAGED LOCKABLE Object { template void SetClass(Class* new_klass) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - Object* GetBrooksPointer() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - void SetBrooksPointer(Object* brooks_pointer) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - void AssertSelfBrooksPointer() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + Object* GetReadBarrierPointer() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + void SetReadBarrierPointer(Object* rb_pointer) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + void AssertReadBarrierPointer() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); // The verifier treats all interfaces as java.lang.Object and relies on runtime checks in // invoke-interface to detect incompatible interface types. @@ -289,12 +289,12 @@ class MANAGED LOCKABLE Object { // Monitor and hash code information. uint32_t monitor_; -#ifdef USE_BROOKS_POINTER - // Note names use a 'x' prefix and the x_brooks_ptr_ is of type int +#ifdef USE_BAKER_OR_BROOKS_READ_BARRIER + // Note names use a 'x' prefix and the x_rb_ptr_ is of type int // instead of Object to go with the alphabetical/by-type field order // on the Java side. - uint32_t x_brooks_ptr_; // For the Brooks pointer. - uint32_t x_padding_; // For 8-byte alignment. TODO: get rid of this. + uint32_t x_rb_ptr_; // For the Baker or Brooks pointer. + uint32_t x_xpadding_; // For 8-byte alignment. TODO: get rid of this. #endif friend class art::ImageWriter; diff --git a/runtime/brooks_pointer.h b/runtime/read_barrier.h similarity index 56% rename from runtime/brooks_pointer.h rename to runtime/read_barrier.h index 3dac6e903..ba0d83042 100644 --- a/runtime/brooks_pointer.h +++ b/runtime/read_barrier.h @@ -14,14 +14,24 @@ * limitations under the License. */ -#ifndef ART_RUNTIME_BROOKS_POINTER_H_ -#define ART_RUNTIME_BROOKS_POINTER_H_ +#ifndef ART_RUNTIME_READ_BARRIER_H_ +#define ART_RUNTIME_READ_BARRIER_H_ // This is in a separate file (from globals.h) because asm_support.h // (a C header, not C++) can't include globals.h. -// Uncomment this and the two fields in Object.java (libcore) to -// enable brooks pointers. -// #define USE_BROOKS_POINTER +// Uncomment one of the following two and the two fields in +// Object.java (libcore) to enable baker or brooks pointers. -#endif // ART_RUNTIME_BROOKS_POINTER_H_ +// #define USE_BAKER_READ_BARRIER +// #define USE_BROOKS_READ_BARRIER + +#if defined(USE_BAKER_READ_BARRIER) || defined(USE_BROOKS_READ_BARRIER) +#define USE_BAKER_OR_BROOKS_READ_BARRIER +#endif + +#if defined(USE_BAKER_READ_BARRIER) && defined(USE_BROOKS_READ_BARRIER) +#error "Only one of Baker or Brooks can be enabled at a time." +#endif + +#endif // ART_RUNTIME_READ_BARRIER_H_ -- 2.11.0