commit f2a69313826f793a7ebeb5ed09ba002904495fc9
author Lokesh Gidra <lokeshgidra@google.com> Tue Mar 27 18:48:59 2018 -0700
committer Lokesh Gidra <lokeshgidra@google.com> Thu Mar 29 05:05:07 2018 -0700
tree 5b84956dd750bb07842f26e4fb9e6cd03a6aa87e
parent 4bf07f6921384d44987d68e6a6cabf32b9b90714

Fix double accounting of skipped objects during copying

When an object is added to skipped_blocks_map_ due to thread
losing the race on installing forwarding pointer during copying,
we add the size of the object to num_bytes_allocated_. Later,
when an object is allocated out of skipped_blocks_map_,
num_bytes_allocated_ is not adjusted accordingly. So, when this
object gets allocated (or gets added to skipped_blocks_map_
again due to losing the race), num_bytes_allocated_ goes off.

Bug: 74763563
Test: make test-art
Change-Id: I197b94b317499281e379733d53ad19794dbe772a

runtime/gc/collector/concurrent_copying.cc

diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index bb5167f..7d35b29 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -2357,14 +2357,13 @@
   size_t non_moving_space_bytes_allocated = 0U;
   size_t bytes_allocated = 0U;
   size_t dummy;
+  bool fall_back_to_non_moving = false;
   mirror::Object* to_ref = region_space_->AllocNonvirtual</*kForEvac*/ true>(
       region_space_alloc_size, &region_space_bytes_allocated, nullptr, &dummy);
   bytes_allocated = region_space_bytes_allocated;
-  if (to_ref != nullptr) {
+  if (LIKELY(to_ref != nullptr)) {
     DCHECK_EQ(region_space_alloc_size, region_space_bytes_allocated);
-  }
-  bool fall_back_to_non_moving = false;
-  if (UNLIKELY(to_ref == nullptr)) {
+  } else {
     // Failed to allocate in the region space. Try the skipped blocks.
     to_ref = AllocateInSkippedBlock(region_space_alloc_size);
     if (to_ref != nullptr) {
@@ -2374,6 +2373,9 @@
         region_space_->RecordAlloc(to_ref);
       }
       bytes_allocated = region_space_alloc_size;
+      heap_->num_bytes_allocated_.fetch_sub(bytes_allocated, std::memory_order_seq_cst);
+      to_space_bytes_skipped_.fetch_sub(bytes_allocated, std::memory_order_seq_cst);
+      to_space_objects_skipped_.fetch_sub(1, std::memory_order_seq_cst);
     } else {
       // Fall back to the non-moving space.
       fall_back_to_non_moving = true;
@@ -2383,7 +2385,6 @@
                   << " skipped_objects="
                   << to_space_objects_skipped_.load(std::memory_order_seq_cst);
       }
-      fall_back_to_non_moving = true;
       to_ref = heap_->non_moving_space_->Alloc(Thread::Current(), obj_size,
                                                &non_moving_space_bytes_allocated, nullptr, &dummy);
       if (UNLIKELY(to_ref == nullptr)) {