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

This reverts commit ea269f69d05fe333e4b36634b925c3c40fc8ce95.

Bug: 355291033
Bug: 354087169
Bug: 360363656
Bug: 361327909
Bug: 364629185

Reason for revert: Failure on asan:
+addressOf succeeded on movable object
+Unexpectedly got 0 address in checkMovable

Change-Id: I1f27845bf3fb7a1542f24b943d999f2d4da5e23a

24 files changed, 67 insertions, 359 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 9342578d42..9bed9833b7 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";
   }
@@ -2381,7 +2380,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);
@@ -2389,11 +2388,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);
@@ -2498,15 +2493,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.
@@ -2517,6 +2504,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()) {
@@ -2533,18 +2521,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.
@@ -2573,12 +2549,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);
@@ -3771,62 +3741,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 13af888204..20a74a475a 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -268,32 +268,19 @@ class Heap {
                                         ObjPtr<mirror::Class> klass,
                                         size_t num_bytes,
                                         const PreFenceVisitor& pre_fence_visitor)
-      REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!*gc_complete_lock_,
-                                                     !*pending_task_lock_,
-                                                     !*backtrace_lock_,
-                                                     !stray_objects_lock_,
-                                                     !process_state_update_lock_,
-                                                     !Roles::uninterruptible_) {
-    bool checked_large_object =
-        (kIsDebugBuild && ShouldAllocLargeObject(klass, num_bytes)) ? true : false;
-
+      REQUIRES_SHARED(Locks::mutator_lock_)
+      REQUIRES(!*gc_complete_lock_,
+               !*pending_task_lock_,
+               !*backtrace_lock_,
+               !process_state_update_lock_,
+               !Roles::uninterruptible_) {
     mirror::Object* obj = AllocObjectWithAllocator<kInstrumented>(self,
                                                                   klass,
                                                                   num_bytes,
                                                                   GetCurrentNonMovingAllocator(),
                                                                   pre_fence_visitor);
-    space::LargeObjectSpace* los = GetLargeObjectsSpace();
-    if (los != nullptr && los->Contains(obj)) {
-      // 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(checked_large_object);
-      AddStrayNonMovableObject(obj);
-    } else {
-      DCHECK(!checked_large_object || los == nullptr);
-    }
     // Java Heap Profiler check and sample allocation.
     if (GetHeapSampler().IsEnabled()) {
-      // TODO (b/365184044) This seems to double sample for large objects?
       JHPCheckNonTlabSampleAllocation(self, obj, num_bytes);
     }
     return obj;
@@ -396,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_);
@@ -718,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_)
@@ -1061,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;
@@ -1131,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);
@@ -1389,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.
@@ -1803,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 1a29619430..0e9660aaac 100644
--- a/runtime/native/dalvik_system_VMRuntime.cc
+++ b/runtime/native/dalvik_system_VMRuntime.cc
@@ -21,12 +21,13 @@
 #include <sys/time.h>
 extern "C" void android_set_application_target_sdk_version(uint32_t version);
 #endif
-#include <android-base/stringprintf.h>
-#include <android-base/strings.h>
 #include <inttypes.h>
+#include <limits>
 #include <limits.h>
+#include "nativehelper/scoped_utf_chars.h"
 
-#include <limits>
+#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
 
 #include "android-base/properties.h"
 #include "arch/instruction_set.h"
@@ -45,7 +46,6 @@ extern "C" void android_set_application_target_sdk_version(uint32_t version);
 #include "gc/heap.h"
 #include "gc/space/dlmalloc_space.h"
 #include "gc/space/image_space.h"
-#include "gc/space/large_object_space.h"
 #include "gc/task_processor.h"
 #include "intern_table.h"
 #include "jit/jit.h"
@@ -58,7 +58,6 @@ extern "C" void android_set_application_target_sdk_version(uint32_t version);
 #include "native_util.h"
 #include "nativehelper/jni_macros.h"
 #include "nativehelper/scoped_local_ref.h"
-#include "nativehelper/scoped_utf_chars.h"
 #include "runtime.h"
 #include "scoped_fast_native_object_access-inl.h"
 #include "scoped_thread_state_change-inl.h"
@@ -116,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();
-  ObjPtr<mirror::Array> result = mirror::Array::Alloc(
-      soa.Self(), array_class, length, array_class->GetComponentSizeShift(), allocator);
-  gc::space::LargeObjectSpace* los = heap->GetLargeObjectsSpace();
-  if (los != nullptr && los->Contains(result.Ptr())) {
-    // We cannot easily avoid this by leaving these in the NonMovable space.
-    // Empirically it is important to unmap these promptly once collected. (b/360363656)
-    heap->AddStrayNonMovableObject(result.Ptr());
-  }
-  // TODO (b/365184044) This fails to sample small objects in JHP?
+  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);
 }
 
@@ -173,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 6cc768cee2..e43d1d3432 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -1206,8 +1206,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();
diff --git a/test/2280-address-of/expected-stderr.txt b/test/2280-address-of/expected-stderr.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/2280-address-of/expected-stderr.txt
+++ /dev/null
diff --git a/test/2280-address-of/expected-stdout.txt b/test/2280-address-of/expected-stdout.txt
deleted file mode 100644
index a965a70ed4..0000000000
--- a/test/2280-address-of/expected-stdout.txt
+++ /dev/null
@@ -1 +0,0 @@
-Done
diff --git a/test/2280-address-of/info.txt b/test/2280-address-of/info.txt
deleted file mode 100644
index 5279e0098a..0000000000
--- a/test/2280-address-of/info.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-Check for correct interaction of nonmovable allocation, clone and addressOf().
-
-Motivated by b/363849521.
diff --git a/test/2280-address-of/src-art/Main.java b/test/2280-address-of/src-art/Main.java
deleted file mode 100644
index a9a918310a..0000000000
--- a/test/2280-address-of/src-art/Main.java
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * Copyright (C) 2024 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.
- */
-
-import dalvik.system.VMRuntime;
-
-public class Main {
-  private static final boolean SHOULD_PRINT = false;  // True causes failure.
-
-  static VMRuntime runtime = VMRuntime.getRuntime();
-
-  static boolean objects_can_move = false;  // Usually set to true in main.
-
-  public static void main(String[] args) throws Exception {
-    Object[] x = new Object[12];
-    int[] int_array = new int[10];
-    int[] large_int_array = new int[100_000];
-
-    try {
-      checkNonMovable(int_array);
-    } catch (RuntimeException e) {
-      if (!e.getMessage().contains(" movable ")) {
-        throw e;
-      }
-      objects_can_move = true;
-    }
-    // Objects_can_move will be true for all collectors we care about.
-    for (int i = 0; i < 10; ++i) {
-      if (i != 0) {
-        for (Object y : x) {
-          checkNonMovable(y);
-        }
-      }
-      checkMovable(int_array);
-      checkMovable(large_int_array);
-      // For completeness, we test various primitive types.
-      // `addressOf` doesn't work for reference types, so we skip those here.
-      // We check both large-object-space and regular objects.
-      x[0] = runtime.newNonMovableArray(int.class, 100);
-      x[1] = runtime.newNonMovableArray(int.class, 100_000);
-      x[2] = runtime.newNonMovableArray(byte.class, 100);
-      x[3] = runtime.newNonMovableArray(byte.class, 100_000);
-      x[4] = runtime.newNonMovableArray(long.class, 100);
-      x[5] = runtime.newNonMovableArray(long.class, 100_000);
-      x[6] = runtime.newNonMovableArray(char.class, 100);
-      x[7] = runtime.newNonMovableArray(char.class, 100_000);
-      x[8] = runtime.newNonMovableArray(float.class, 100);
-      x[9] = runtime.newNonMovableArray(float.class, 100_000);
-      x[10] = runtime.newNonMovableArray(double.class, 100);
-      x[11] = runtime.newNonMovableArray(double.class, 100_000);
-      for (Object y : x) {
-        checkNonMovable(y);
-      }
-      if (i % 2 == 1) {
-        // Doesn't compile without casts because Object.clone() is protected.
-        x[0] = ((int[])x[0]).clone();
-        x[1] = ((int[])x[1]).clone();
-        x[2] = ((byte[])x[2]).clone();
-        x[3] = ((byte[])x[3]).clone();
-        x[4] = ((long[])x[4]).clone();
-        x[5] = ((long[])x[5]).clone();
-        int_array = int_array.clone();
-      } else {
-        x[6] = ((char[])x[6]).clone();
-        x[7] = ((char[])x[7]).clone();
-        x[8] = ((float[])x[8]).clone();
-        x[9] = ((float[])x[9]).clone();
-        x[10] = ((double[])x[10]).clone();
-        x[11] = ((double[])x[11]).clone();
-        large_int_array = large_int_array.clone();
-      }
-      System.gc();
-      System.runFinalization();
-    }
-    System.out.println("Done");
-  }
-
-  public static void checkNonMovable(Object x) {
-    // If things go wrong, this will throw instead. A zero return would be very strange indeed.
-    if (runtime.addressOf(x) == 0) {
-      System.out.println("Unexpectedly got 0 address");
-    }
-  }
-
-  public static void checkMovable(Object x) {
-    if (!objects_can_move) {
-      return;
-    }
-    try {
-      if (runtime.addressOf(x) != 0) {
-        System.out.println("addressOf succeeded on movable object");
-      }
-      System.out.println("Unexpectedly got 0 address in checkMovable");
-    } catch (RuntimeException e) { /* expected */ }
-  }
-}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index eb12db4a9f..e58a6cd4a2 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1236,8 +1236,7 @@
                   "2263-method-trace-jit",
                   "2270-mh-internal-hiddenapi-use",
                   "2271-profile-inline-cache",
-                  "2279-aconfig-flags",
-                  "2280-address-of"],
+                  "2279-aconfig-flags"],
         "variant": "jvm",
         "description": ["Doesn't run on RI."]
     },