Revert^3 "Object.clone() allocates more movable objects"

This reverts commit 1956542906d0b128a86975af009ba4601d02129a.

Reason for revert: Droid Monitor created revert due to this b/363849521. 

Change-Id: Ic56e46bc11b1fd696b82f29782a1d1735ee805a9

19 files changed, 56 insertions, 221 deletions

diff --git a/compiler/optimizing/intrinsics.cc b/compiler/optimizing/intrinsics.cc
index 8abe6a4563..06ea1c6ffb 100644
--- a/compiler/optimizing/intrinsics.cc
+++ b/compiler/optimizing/intrinsics.cc
@@ -218,7 +218,7 @@ void IntrinsicVisitor::AssertNonMovableStringClass() {
   if (kIsDebugBuild) {
     ScopedObjectAccess soa(Thread::Current());
     ObjPtr<mirror::Class> string_class = GetClassRoot<mirror::String>();
-    CHECK(!art::Runtime::Current()->GetHeap()->ObjectMayMove(string_class));
+    CHECK(!art::Runtime::Current()->GetHeap()->IsMovableObject(string_class));
   }
 }
 
diff --git a/libartbase/base/bit_vector.h b/libartbase/base/bit_vector.h
index 2eed9702bf..ec94efb09f 100644
--- a/libartbase/base/bit_vector.h
+++ b/libartbase/base/bit_vector.h
@@ -31,9 +31,9 @@ class ArenaBitVector;
 
 /*
  * Expanding bitmap. Bits are numbered starting from zero. All operations on a BitVector are
- * unsynchronized. New BitVectors are not necessarily zeroed out. If the used allocator doesn't
- * clear the vector (e.g. ScopedArenaAllocator), the caller is responsible for clearing it (e.g.
- * ArenaBitVector).
+ * unsynchronized. New BitVectors are not necessarily zeroed out. If the used allocator doesn't do
+ * clear the vector (e.g. ScopedArenaAllocator), the responsibility of clearing it relies on the
+ * caller (e.g. ArenaBitVector).
  */
 class BitVector {
  public:
diff --git a/openjdkjvmti/ti_heap.cc b/openjdkjvmti/ti_heap.cc
index f8589f1d1a..80bfa0ff43 100644
--- a/openjdkjvmti/ti_heap.cc
+++ b/openjdkjvmti/ti_heap.cc
@@ -1916,10 +1916,6 @@ jvmtiError HeapExtensions::ChangeArraySize(jvmtiEnv* env, jobject arr, jsize new
   art::StackHandleScope<2> hs(self);
   art::Handle<art::mirror::Array> old_arr(hs.NewHandle(soa.Decode<art::mirror::Array>(arr)));
   art::MutableHandle<art::mirror::Array> new_arr(hs.NewHandle<art::mirror::Array>(nullptr));
-  if (!art::Runtime::Current()->GetHeap()->PossiblyAllocatedMovable(old_arr.Get())) {
-    JVMTI_LOG(INFO, env) << "Cannot resize a nonmovable array";
-    return ERR(ILLEGAL_ARGUMENT);
-  }
   if (klass->IsObjectArrayClass()) {
     new_arr.Assign(
         art::mirror::ObjectArray<art::mirror::Object>::Alloc(self, old_arr->GetClass(), new_size));
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index fafa374a61..e3d89e7449 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -526,10 +526,9 @@ class SCOPED_CAPABILITY MutexLock {
 
 // Pretend to acquire a mutex for checking purposes, without actually doing so. Use with
 // extreme caution when it is known the condition that the mutex would guard against cannot arise.
-template <typename MutexType>
 class SCOPED_CAPABILITY FakeMutexLock {
  public:
-  explicit FakeMutexLock(MutexType& mu) ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {}
+  explicit FakeMutexLock(Mutex& mu) ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {}
 
   ~FakeMutexLock() RELEASE() NO_THREAD_SAFETY_ANALYSIS {}
 
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index dfcc2664f7..ad3e690938 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -377,10 +377,11 @@ Heap::Heap(size_t initial_size,
        * verification is enabled, we limit the size of allocation stacks to speed up their
        * searching.
        */
-      max_allocation_stack_size_(kGCALotMode ? kGcAlotAllocationStackSize :
-                                 (kVerifyObjectSupport > kVerifyObjectModeFast) ?
-                                               kVerifyObjectAllocationStackSize :
-                                               kDefaultAllocationStackSize),
+      max_allocation_stack_size_(kGCALotMode
+          ? kGcAlotAllocationStackSize
+          : (kVerifyObjectSupport > kVerifyObjectModeFast)
+              ? kVerifyObjectAllocationStackSize
+              : kDefaultAllocationStackSize),
       current_allocator_(kAllocatorTypeDlMalloc),
       current_non_moving_allocator_(kAllocatorTypeNonMoving),
       bump_pointer_space_(nullptr),
@@ -431,9 +432,7 @@ Heap::Heap(size_t initial_size,
       boot_image_spaces_(),
       boot_images_start_address_(0u),
       boot_images_size_(0u),
-      pre_oome_gc_count_(0u),
-      stray_non_movable_objects_(),
-      stray_objects_lock_("lock for stray_non_movable_objects", kGenericBottomLock) {
+      pre_oome_gc_count_(0u) {
   if (VLOG_IS_ON(heap) || VLOG_IS_ON(startup)) {
     LOG(INFO) << "Heap() entering";
   }
@@ -2379,7 +2378,7 @@ class ZygoteCompactingCollector final : public collector::SemiSpace {
     bin_live_bitmap_ = space->GetLiveBitmap();
     bin_mark_bitmap_ = space->GetMarkBitmap();
     uintptr_t prev = reinterpret_cast<uintptr_t>(space->Begin());
-    Heap* heap = Runtime::Current()->GetHeap();
+    WriterMutexLock mu(Thread::Current(), *Locks::heap_bitmap_lock_);
     // Note: This requires traversing the space in increasing order of object addresses.
     auto visitor = [&](mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_) {
       uintptr_t object_addr = reinterpret_cast<uintptr_t>(obj);
@@ -2387,11 +2386,7 @@ class ZygoteCompactingCollector final : public collector::SemiSpace {
       // Add the bin consisting of the end of the previous object to the start of the current object.
       AddBin(bin_size, prev);
       prev = object_addr + RoundUp(obj->SizeOf<kDefaultVerifyFlags>(), kObjectAlignment);
-      if (!obj->IsClass()) {
-        heap->AddStrayNonMovableObject(obj);
-      }
     };
-    WriterMutexLock mu(Thread::Current(), *Locks::heap_bitmap_lock_);
     bin_live_bitmap_->Walk(visitor);
     // Add the last bin which spans after the last object to the end of the space.
     AddBin(reinterpret_cast<uintptr_t>(space->End()) - prev, prev);
@@ -2496,15 +2491,7 @@ void Heap::IncrementFreedEver() {
 // This has a large frame, but shouldn't be run anywhere near the stack limit.
 // FIXME: BUT it did exceed... http://b/197647048
 #  pragma clang diagnostic ignored "-Wframe-larger-than="
-void Heap::PreZygoteFork() NO_THREAD_SAFETY_ANALYSIS /* TODO: Why? */ {
-  Thread* self = Thread::Current();
-  // Opportunistically log here; empirically logs from the initial PreZygoteFork() are lost.
-  // But for the main zygote, this is typically entered at least twice.
-  {
-    MutexLock(self, stray_objects_lock_);
-    LOG(INFO) << "PreZygoteFork(): stray_non_movable_objects_.size() = "
-              << stray_non_movable_objects_.size();
-  }
+void Heap::PreZygoteFork() {
   if (!HasZygoteSpace()) {
     // We still want to GC in case there is some unreachable non moving objects that could cause a
     // suboptimal bin packing when we compact the zygote space.
@@ -2515,6 +2502,7 @@ void Heap::PreZygoteFork() NO_THREAD_SAFETY_ANALYSIS /* TODO: Why? */ {
   }
   // We need to close userfaultfd fd for app/webview zygotes to avoid getattr
   // (stat) on the fd during fork.
+  Thread* self = Thread::Current();
   MutexLock mu(self, zygote_creation_lock_);
   // Try to see if we have any Zygote spaces.
   if (HasZygoteSpace()) {
@@ -2531,18 +2519,6 @@ void Heap::PreZygoteFork() NO_THREAD_SAFETY_ANALYSIS /* TODO: Why? */ {
   // there.
   non_moving_space_->GetMemMap()->Protect(PROT_READ | PROT_WRITE);
   const bool same_space = non_moving_space_ == main_space_;
-  // We create the ZygoteSpace by performing a semi-space collection to copy the main allocation
-  // space into what was the non-moving space. We do so by ignoring and overwriting the meta-
-  // information from the non-moving (dlmalloc) space. An initial pass identifies unused sections
-  // of the heap that we usually try to copy into first. We copy any remaining objects past the
-  // previous end of the old non-moving space. Eeverything up to the last allocated object in the
-  // old non-moving space then becomes ZygoteSpace. Everything after that becomes the new
-  // non-moving space.
-  // There is a subtlety here in that Object.clone() treats objects allocated as non-movable
-  // differently from other objects, and this ZygoteSpace creation process doesn't automatically
-  // preserve that distinction. Thus we must explicitly track this in stray_non_movable_objects_.
-  // Otherwise we have to treat the entire ZygoteSpace as non-movable, which could cause some
-  // weird programming styles to eventually render most of the heap non-movable.
   if (kCompactZygote) {
     // Temporarily disable rosalloc verification because the zygote
     // compaction will mess up the rosalloc internal metadata.
@@ -2571,12 +2547,6 @@ void Heap::PreZygoteFork() NO_THREAD_SAFETY_ANALYSIS /* TODO: Why? */ {
     zygote_collector.SetToSpace(&target_space);
     zygote_collector.SetSwapSemiSpaces(false);
     zygote_collector.Run(kGcCauseCollectorTransition, false);
-    {
-      MutexLock(self, stray_objects_lock_);
-      uint32_t num_nonmovable = stray_non_movable_objects_.size();
-      // For an AOSP boot, we saw num_nonmovable around a dozen.
-      DCHECK_LT(num_nonmovable, 1000u) << " Too many nonmovable zygote objects?";
-    }
     if (reset_main_space) {
       main_space_->GetMemMap()->Protect(PROT_READ | PROT_WRITE);
       madvise(main_space_->Begin(), main_space_->Capacity(), MADV_DONTNEED);
@@ -3766,62 +3736,7 @@ void Heap::SetIdealFootprint(size_t target_footprint) {
   target_footprint_.store(target_footprint, std::memory_order_relaxed);
 }
 
-void Heap::AddStrayNonMovableObject(mirror::Object* obj) {
-  MutexLock mu(Thread::Current(), stray_objects_lock_);
-  stray_non_movable_objects_.insert(
-      mirror::CompressedReference<mirror::Object>::FromMirrorPtr(obj));
-  if (kIsDebugBuild) {
-    size_t current_size = stray_non_movable_objects_.size();
-    static size_t last_reported_size /* GUARDED_BY(stray_objects_lock_) */ = 0;
-    if (current_size % 100 == 0 && current_size > last_reported_size) {
-      LOG(WARNING) << "stray_non_movable_objects_.size() = " << current_size;
-      last_reported_size = current_size;
-    }
-  }
-}
-
-void Heap::RemoveStrayNonMovableObject(mirror::Object* obj) {
-  MutexLock mu(Thread::Current(), stray_objects_lock_);
-  // FromMirrorPtr normally needs the mutator lock. But we only traffic unreachable and
-  // nonmovable objects, so we don't care.
-  FakeMutexLock mu2(*Locks::mutator_lock_);
-  auto iter = stray_non_movable_objects_.find(
-      mirror::CompressedReference<mirror::Object>::FromMirrorPtr(obj));
-  if (iter != stray_non_movable_objects_.end()) {
-    stray_non_movable_objects_.erase(iter);
-  }
-}
-
-bool Heap::IsNonMovable(ObjPtr<mirror::Object> obj) const {
-  DCHECK(!obj.Ptr()->IsClass());  // We do not correctly track classes in zygote space.
-  if (GetNonMovingSpace()->Contains(obj.Ptr())) {
-    return true;
-  }
-  if ((zygote_space_ != nullptr && zygote_space_->Contains(obj.Ptr())) ||
-      (GetLargeObjectsSpace() != nullptr && GetLargeObjectsSpace()->Contains(obj.Ptr()))) {
-    MutexLock mu(Thread::Current(), stray_objects_lock_);
-    return stray_non_movable_objects_.contains(
-        mirror::CompressedReference<mirror::Object>::FromMirrorPtr(obj.Ptr()));
-  }
-  return false;  // E.g. in the main moving space.
-}
-
-bool Heap::PossiblyAllocatedMovable(ObjPtr<mirror::Object> obj) const {
-  // The CC collector may copy movable objects into NonMovingSpace. It does that only when it
-  // runs out of space, so we assume this does not affect ZygoteSpace.
-  if (!gUseReadBarrier && GetNonMovingSpace()->Contains(obj.Ptr())) {
-    return false;
-  }
-  if ((zygote_space_ != nullptr && zygote_space_->Contains(obj.Ptr())) ||
-      (GetLargeObjectsSpace() != nullptr && GetLargeObjectsSpace()->Contains(obj.Ptr()))) {
-    MutexLock mu(Thread::Current(), stray_objects_lock_);
-    return !stray_non_movable_objects_.contains(
-        mirror::CompressedReference<mirror::Object>::FromMirrorPtr(obj.Ptr()));
-  }
-  return true;
-}
-
-bool Heap::ObjectMayMove(ObjPtr<mirror::Object> obj) const {
+bool Heap::IsMovableObject(ObjPtr<mirror::Object> obj) const {
   if (kMovingCollector) {
     space::Space* space = FindContinuousSpaceFromObject(obj.Ptr(), true);
     if (space != nullptr) {
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index f0c002f749..58a73af74b 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -383,29 +383,8 @@ class Heap {
                           bool sorted = false)
       REQUIRES_SHARED(Locks::heap_bitmap_lock_, Locks::mutator_lock_);
 
-  // Approximates whether the object in question was explicitly requested to be nonmovable.
-  // May rarely err on the side of claiming immovability for objects that were allocated movable,
-  // but will not be moved.
-  // Returns true if and only if one of the following is true:
-  // 1) The object was allocated as nonmovable, whether or not it has been redirected to
-  //    ZygoteSpace or LargeObjectsSpace.
-  // 2) All objects are being allocated in a non-movable space.
-  // 3) The CC collector decided to spuriously allocate in non-moving space because it ran
-  //    out of memory at an inopportune time.
-  // This is used primarily to determine Object.clone() behavior, where (2)
-  // doesn't matter. (3) is unfortunate, but we can live with it.
-  // SHOULD NOT BE CALLED ON CLASS OBJECTS.
-  bool IsNonMovable(ObjPtr<mirror::Object> obj) const REQUIRES_SHARED(Locks::mutator_lock_);
-
-  // The negation of the above, but resolves ambiguous cases in the direction of assuming
-  // movability. Used for partial error checking where an object must be movable.
-  EXPORT bool PossiblyAllocatedMovable(ObjPtr<mirror::Object> obj) const
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
-  // Returns true if there is any chance that the object (obj) will move. Returns false for image
-  // and zygote space, since we don't actually move objects in those spaces. Unlike the preceding
-  // function, the result here depends on whether the object was moved to zygote or image space.
-  bool ObjectMayMove(ObjPtr<mirror::Object> obj) const REQUIRES_SHARED(Locks::mutator_lock_);
+  // Returns true if there is any chance that the object (obj) will move.
+  bool IsMovableObject(ObjPtr<mirror::Object> obj) const REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Enables us to compacting GC until objects are released.
   EXPORT void IncrementDisableMovingGC(Thread* self) REQUIRES(!*gc_complete_lock_);
@@ -705,7 +684,7 @@ class Heap {
     return allocation_stack_.get();
   }
 
-  void PreZygoteFork();
+  void PreZygoteFork() NO_THREAD_SAFETY_ANALYSIS;
 
   // Mark and empty stack.
   EXPORT void FlushAllocStack() REQUIRES_SHARED(Locks::mutator_lock_)
@@ -1048,18 +1027,6 @@ class Heap {
     return size < pud_size ? pmd_size : pud_size;
   }
 
-  // Add a reference to the set of preexisting zygote and LOS nonmovable objects.
-  void AddStrayNonMovableObject(mirror::Object* obj) REQUIRES(!stray_objects_lock_)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
-  // Remove a reference from the set of preexisting zygote and LOS nonmovable objects.  Do nothing
-  // if it was not present. The argument should no longer be accessible to the mutator or
-  // subsequent GCs.
-  void RemoveStrayNonMovableObject(mirror::Object* obj) REQUIRES(!stray_objects_lock_);
-
-  bool ShouldAllocLargeObject(ObjPtr<mirror::Class> c, size_t byte_count) const
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
  private:
   class ConcurrentGCTask;
   class CollectorTransitionTask;
@@ -1118,6 +1085,8 @@ class Heap {
         collector_type == kCollectorTypeCMCBackground ||
         collector_type == kCollectorTypeHomogeneousSpaceCompact;
   }
+  bool ShouldAllocLargeObject(ObjPtr<mirror::Class> c, size_t byte_count) const
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Checks whether we should garbage collect:
   ALWAYS_INLINE bool ShouldConcurrentGCForJava(size_t new_num_bytes_allocated);
@@ -1376,7 +1345,7 @@ class Heap {
   std::vector<space::AllocSpace*> alloc_spaces_;
 
   // A space where non-movable objects are allocated, when compaction is enabled it contains
-  // Classes, and non moving objects.
+  // Classes, ArtMethods, ArtFields, and non moving objects.
   space::MallocSpace* non_moving_space_;
 
   // Space which we use for the kAllocatorTypeROSAlloc.
@@ -1790,19 +1759,6 @@ class Heap {
 
   std::unique_ptr<Verification> verification_;
 
-  // Non-class immovable objects allocated in zygote soace or LOS instead of the nonmovable object
-  // space.
-  // TODO: We may need a smaller data structure. Unfortunately, HashSets minimum size is too big.
-  struct CRComparator {
-    bool operator()(mirror::CompressedReference<mirror::Object> x,
-                    mirror::CompressedReference<mirror::Object> y) const {
-      return x.AsVRegValue() < y.AsVRegValue();
-    }
-  };
-  std::set<mirror::CompressedReference<mirror::Object>, CRComparator> stray_non_movable_objects_
-      GUARDED_BY(stray_objects_lock_);
-  mutable Mutex stray_objects_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
-
   friend class CollectorTransitionTask;
   friend class collector::GarbageCollector;
   friend class collector::ConcurrentCopying;
diff --git a/runtime/gc/space/large_object_space.cc b/runtime/gc/space/large_object_space.cc
index e01c8ee3ae..a4b9d24184 100644
--- a/runtime/gc/space/large_object_space.cc
+++ b/runtime/gc/space/large_object_space.cc
@@ -201,7 +201,6 @@ size_t LargeObjectMapSpace::Free(Thread* self, mirror::Object* ptr) {
     Runtime::Current()->GetHeap()->DumpSpaces(LOG_STREAM(FATAL_WITHOUT_ABORT));
     LOG(FATAL) << "Attempted to free large object " << ptr << " which was not live";
   }
-  Runtime::Current()->GetHeap()->RemoveStrayNonMovableObject(ptr);
   const size_t map_size = it->second.mem_map.BaseSize();
   DCHECK_GE(num_bytes_allocated_, map_size);
   size_t allocation_size = map_size;
@@ -466,8 +465,6 @@ size_t FreeListSpace::Free(Thread* self, mirror::Object* obj) {
   DCHECK_GT(allocation_size, 0U);
   DCHECK_ALIGNED_PARAM(allocation_size, ObjectAlignment());
 
-  Runtime::Current()->GetHeap()->RemoveStrayNonMovableObject(obj);
-
   // madvise the pages without lock
   madvise(obj, allocation_size, MADV_DONTNEED);
   if (kIsDebugBuild) {
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 1b7cf7d76c..2b83eff44f 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -397,7 +397,7 @@ static void DCheckRootsAreValid(const std::vector<Handle<mirror::Object>>& roots
     }
     // Ensure that we don't put movable objects in the shared region.
     if (is_shared_region) {
-      CHECK(Runtime::Current()->GetHeap()->IsNonMovable(object.Get()));
+      CHECK(!Runtime::Current()->GetHeap()->IsMovableObject(object.Get()));
     }
   }
 }
diff --git a/runtime/jni/jni_internal.cc b/runtime/jni/jni_internal.cc
index 48428f6c3d..51350dc713 100644
--- a/runtime/jni/jni_internal.cc
+++ b/runtime/jni/jni_internal.cc
@@ -2126,7 +2126,7 @@ class JNI {
     ScopedObjectAccess soa(env);
     ObjPtr<mirror::String> s = soa.Decode<mirror::String>(java_string);
     gc::Heap* heap = Runtime::Current()->GetHeap();
-    if (heap->ObjectMayMove(s) || s->IsCompressed()) {
+    if (heap->IsMovableObject(s) || s->IsCompressed()) {
       jchar* chars = new jchar[s->GetLength()];
       if (s->IsCompressed()) {
         int32_t length = s->GetLength();
@@ -2174,7 +2174,7 @@ class JNI {
       }
       return chars;
     } else {
-      if (heap->ObjectMayMove(s)) {
+      if (heap->IsMovableObject(s)) {
         StackHandleScope<1> hs(soa.Self());
         HandleWrapperObjPtr<mirror::String> h(hs.NewHandleWrapper(&s));
         if (!gUseReadBarrier && !gUseUserfaultfd) {
@@ -2202,7 +2202,7 @@ class JNI {
     ScopedObjectAccess soa(env);
     gc::Heap* heap = Runtime::Current()->GetHeap();
     ObjPtr<mirror::String> s = soa.Decode<mirror::String>(java_string);
-    if (!s->IsCompressed() && heap->ObjectMayMove(s)) {
+    if (!s->IsCompressed() && heap->IsMovableObject(s)) {
       if (!gUseReadBarrier && !gUseUserfaultfd) {
         heap->DecrementDisableMovingGC(soa.Self());
       } else {
@@ -2369,7 +2369,7 @@ class JNI {
       return nullptr;
     }
     gc::Heap* heap = Runtime::Current()->GetHeap();
-    if (heap->ObjectMayMove(array)) {
+    if (heap->IsMovableObject(array)) {
       if (!gUseReadBarrier && !gUseUserfaultfd) {
         heap->IncrementDisableMovingGC(soa.Self());
       } else {
@@ -2966,7 +2966,7 @@ class JNI {
       return nullptr;
     }
     // Only make a copy if necessary.
-    if (Runtime::Current()->GetHeap()->ObjectMayMove(array)) {
+    if (Runtime::Current()->GetHeap()->IsMovableObject(array)) {
       if (is_copy != nullptr) {
         *is_copy = JNI_TRUE;
       }
@@ -3026,7 +3026,7 @@ class JNI {
     if (mode != JNI_COMMIT) {
       if (is_copy) {
         delete[] reinterpret_cast<uint64_t*>(elements);
-      } else if (heap->ObjectMayMove(array)) {
+      } else if (heap->IsMovableObject(array)) {
         // Non copy to a movable object must means that we had disabled the moving GC.
         if (!gUseReadBarrier && !gUseUserfaultfd) {
           heap->DecrementDisableMovingGC(soa.Self());
diff --git a/runtime/jni/jni_internal_test.cc b/runtime/jni/jni_internal_test.cc
index a575981d9b..ed97e4d4c8 100644
--- a/runtime/jni/jni_internal_test.cc
+++ b/runtime/jni/jni_internal_test.cc
@@ -1829,7 +1829,7 @@ TEST_F(JniInternalTest, GetStringChars_ReleaseStringChars) {
 
   jboolean is_copy = JNI_FALSE;
   chars = env_->GetStringChars(s, &is_copy);
-  if (Runtime::Current()->GetHeap()->ObjectMayMove(s_m)) {
+  if (Runtime::Current()->GetHeap()->IsMovableObject(s_m)) {
     EXPECT_EQ(JNI_TRUE, is_copy);
   } else {
     EXPECT_EQ(JNI_FALSE, is_copy);
diff --git a/runtime/mirror/array-alloc-inl.h b/runtime/mirror/array-alloc-inl.h
index 69d2e2017f..b905fd1727 100644
--- a/runtime/mirror/array-alloc-inl.h
+++ b/runtime/mirror/array-alloc-inl.h
@@ -143,14 +143,16 @@ inline ObjPtr<Array> Array::Alloc(Thread* self,
   ObjPtr<Array> result;
   if (!kFillUsable) {
     SetLengthVisitor visitor(component_count);
-    result = ObjPtr<Array>::DownCast(heap->AllocObjectWithAllocator<kIsInstrumented>(
-        self, array_class, size, allocator_type, visitor));
+    result = ObjPtr<Array>::DownCast(
+        heap->AllocObjectWithAllocator<kIsInstrumented>(
+            self, array_class, size, allocator_type, visitor));
   } else {
     SetLengthToUsableSizeVisitor visitor(component_count,
                                          DataOffset(1U << component_size_shift).SizeValue(),
                                          component_size_shift);
-    result = ObjPtr<Array>::DownCast(heap->AllocObjectWithAllocator<kIsInstrumented>(
-        self, array_class, size, allocator_type, visitor));
+    result = ObjPtr<Array>::DownCast(
+        heap->AllocObjectWithAllocator<kIsInstrumented>(
+            self, array_class, size, allocator_type, visitor));
   }
   if (kIsDebugBuild && result != nullptr && Runtime::Current()->IsStarted()) {
     array_class = result->GetClass();  // In case the array class moved.
diff --git a/runtime/mirror/array.cc b/runtime/mirror/array.cc
index 65371d0f5e..a4f6c88e4c 100644
--- a/runtime/mirror/array.cc
+++ b/runtime/mirror/array.cc
@@ -144,8 +144,9 @@ ObjPtr<Array> Array::CopyOf(Handle<Array> h_this, Thread* self, int32_t new_leng
   CHECK(klass->IsPrimitiveArray()) << "Will miss write barriers";
   DCHECK_GE(new_length, 0);
   auto* heap = Runtime::Current()->GetHeap();
-  DCHECK(!heap->IsNonMovable(h_this.Get()));
-  gc::AllocatorType allocator_type = heap->GetCurrentAllocator();
+  gc::AllocatorType allocator_type = heap->IsMovableObject(h_this.Get())
+      ? heap->GetCurrentAllocator()
+      : heap->GetCurrentNonMovingAllocator();
   const auto component_size = klass->GetComponentSize();
   const auto component_shift = klass->GetComponentSizeShift();
   ObjPtr<Array> new_array =
diff --git a/runtime/mirror/array.h b/runtime/mirror/array.h
index 876fbf061b..7a0976ab48 100644
--- a/runtime/mirror/array.h
+++ b/runtime/mirror/array.h
@@ -50,7 +50,8 @@ class MANAGED Array : public Object {
                                            int32_t component_count,
                                            size_t component_size_shift,
                                            gc::AllocatorType allocator_type)
-      REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Roles::uninterruptible_);
+      REQUIRES_SHARED(Locks::mutator_lock_)
+      REQUIRES(!Roles::uninterruptible_);
 
   static ObjPtr<Array> CreateMultiArray(Thread* self,
                                         Handle<Class> element_class,
diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index 448409c7a7..b28978603c 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -162,11 +162,9 @@ ObjPtr<Object> Object::Clone(Handle<Object> h_this, Thread* self) {
   gc::Heap* heap = Runtime::Current()->GetHeap();
   size_t num_bytes = h_this->SizeOf();
   CopyObjectVisitor visitor(&h_this, num_bytes);
-  // Unclear whether this should ever allocate a nonmovable object. This is conservative.
-  ObjPtr<Object> copy =
-      heap->IsNonMovable(h_this.Get()) ?
-          heap->AllocNonMovableObject(self, h_this->GetClass(), num_bytes, visitor) :
-          heap->AllocObject(self, h_this->GetClass(), num_bytes, visitor);
+  ObjPtr<Object> copy = heap->IsMovableObject(h_this.Get())
+      ? heap->AllocObject(self, h_this->GetClass(), num_bytes, visitor)
+      : heap->AllocNonMovableObject(self, h_this->GetClass(), num_bytes, visitor);
   if (h_this->GetClass()->IsFinalizable()) {
     heap->AddFinalizerReference(self, &copy);
   }
diff --git a/runtime/mirror/object_array-alloc-inl.h b/runtime/mirror/object_array-alloc-inl.h
index d3688762a4..e79d154f84 100644
--- a/runtime/mirror/object_array-alloc-inl.h
+++ b/runtime/mirror/object_array-alloc-inl.h
@@ -66,8 +66,9 @@ inline ObjPtr<ObjectArray<T>> ObjectArray<T>::CopyOf(Handle<ObjectArray<T>> h_th
                                                      int32_t new_length) {
   DCHECK_GE(new_length, 0);
   gc::Heap* heap = Runtime::Current()->GetHeap();
-  DCHECK(heap->PossiblyAllocatedMovable(h_this.Get()));
-  gc::AllocatorType allocator_type = heap->GetCurrentAllocator();
+  gc::AllocatorType allocator_type = heap->IsMovableObject(h_this.Get())
+      ? heap->GetCurrentAllocator()
+      : heap->GetCurrentNonMovingAllocator();
   ObjPtr<ObjectArray<T>> new_array = Alloc(self, h_this->GetClass(), new_length, allocator_type);
   if (LIKELY(new_array != nullptr)) {
     new_array->AssignableMemcpy(0, h_this.Get(), 0, std::min(h_this->GetLength(), new_length));
diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc
index 0ff1ab9da2..3658c9c31c 100644
--- a/runtime/native/dalvik_system_VMRuntime.cc
+++ b/runtime/native/dalvik_system_VMRuntime.cc
@@ -115,17 +115,12 @@ static jobject VMRuntime_newNonMovableArray(JNIEnv* env, jobject, jclass javaEle
   if (UNLIKELY(array_class == nullptr)) {
     return nullptr;
   }
-  gc::Heap* heap = runtime->GetHeap();
-  gc::AllocatorType allocator = heap->GetCurrentNonMovingAllocator();
-  bool is_large = heap->ShouldAllocLargeObject(array_class, length);
-  ObjPtr<mirror::Array> result = mirror::Array::Alloc(
-      soa.Self(), array_class, length, array_class->GetComponentSizeShift(), allocator);
-  if (is_large) {
-    // We cannot easily avoid this by leaving these in the NonMovable space.
-    // Empirically it is important to unmap these promptly once collected. (b/360363656)
-    DCHECK(heap->GetLargeObjectsSpace()->Contains(result.Ptr()));
-    heap->AddStrayNonMovableObject(result.Ptr());
-  }
+  gc::AllocatorType allocator = runtime->GetHeap()->GetCurrentNonMovingAllocator();
+  ObjPtr<mirror::Array> result = mirror::Array::Alloc(soa.Self(),
+                                                      array_class,
+                                                      length,
+                                                      array_class->GetComponentSizeShift(),
+                                                      allocator);
   return soa.AddLocalReference<jobject>(result);
 }
 
@@ -172,11 +167,10 @@ static jlong VMRuntime_addressOf(JNIEnv* env, jobject, jobject javaArray) {
     ThrowIllegalArgumentException("not a primitive array");
     return 0;
   }
-  if (!Runtime::Current()->GetHeap()->IsNonMovable(array)) {
+  if (Runtime::Current()->GetHeap()->IsMovableObject(array)) {
     ThrowRuntimeException("Trying to get address of movable array object");
     return 0;
   }
-  DCHECK(!Runtime::Current()->GetHeap()->ObjectMayMove(array));
   return reinterpret_cast<uintptr_t>(array->GetRawData(array->GetClass()->GetComponentSize(), 0));
 }
 
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 46a73cd1e4..53c1cb152f 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1404,8 +1404,7 @@ static size_t OpenBootDexFiles(ArrayRef<const std::string> dex_filenames,
 void Runtime::SetSentinel(ObjPtr<mirror::Object> sentinel) {
   CHECK(sentinel_.Read() == nullptr);
   CHECK(sentinel != nullptr);
-  // IsNonMovable(sentinel) doesn't hold if it came from an image.
-  CHECK(!heap_->ObjectMayMove(sentinel));
+  CHECK(!heap_->IsMovableObject(sentinel));
   sentinel_ = GcRoot<mirror::Object>(sentinel);
 }
 
diff --git a/runtime/runtime.h b/runtime/runtime.h
index d0a238e0e6..64d85b6464 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -1210,8 +1210,8 @@ class Runtime {
   // for differentiating between unfilled imt slots vs conflict slots in superclasses.
   ArtMethod* imt_unimplemented_method_;
 
-  // Special sentinel object used to indicate invalid conditions in JNI (cleared weak references)
-  // and JDWP (invalid references).
+  // Special sentinel object used to invalid conditions in JNI (cleared weak references) and
+  // JDWP (invalid references).
   GcRoot<mirror::Object> sentinel_;
 
   InstructionSet instruction_set_;
diff --git a/test/070-nio-buffer/src/Main.java b/test/070-nio-buffer/src/Main.java
index 3f4a59db9f..a3eeb3fda6 100644
--- a/test/070-nio-buffer/src/Main.java
+++ b/test/070-nio-buffer/src/Main.java
@@ -32,7 +32,6 @@ public class Main {
         intFloatTest();
         basicShortTest();
         primTest();
-        largeBuffersTest();
     }
 
     /*
@@ -119,29 +118,6 @@ public class Main {
         storeValues(directBuf);
     }
 
-    /*
-     * Check that we can allocate half of maxMemory() in large direct ByteBuffers.
-     * Some apps need this in spite of the fact that we have a lower limit for smaller buffers.
-     */
-    public static void largeBuffersTest() {
-        final int bufSize = 4_000_000;
-        final int nBuffers = (int) (Runtime.getRuntime().maxMemory() / bufSize) / 2;
-        ByteBuffer[] directBufs = new ByteBuffer[nBuffers];
-        for (int i = 0; i < directBufs.length; ++i) {
-            directBufs[i] = ByteBuffer.allocateDirect(bufSize);
-            directBufs[i].asIntBuffer().put(i, i);
-        }
-        for (int i = 0; i < directBufs.length; ++i) {
-            if (directBufs[i].asIntBuffer().get(i) != i) {
-                System.out.println("ERROR: Contents check failed for " + i);
-            }
-            if (directBufs[i].asIntBuffer().get(50) != 0) {
-                System.out.println("ERROR: Zero check failed for buffer " + i);
-            }
-            directBufs[i] = null;
-        }
-    }
-
     static void storeValues(ByteBuffer directBuf) {
         directBuf.position(0);
         ShortBuffer shortBuf = directBuf.asShortBuffer();