[heap] Make SlotSet allocation thread-safe and refactor code.

src/heap/mark-compact.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/heap/mark-compact.h"
 
 #include "src/base/atomicops.h"
 #include "src/base/bits.h"
 #include "src/base/sys-info.h"
 #include "src/code-stubs.h"
@@ skipping 621 matching lines @@
       space->top() == space->limit()
           ? nullptr
           : Page::FromAllocationAreaAddress(space->top());
   for (Page* p : *space) {
     if (p->NeverEvacuate() || p == owner_of_linear_allocation_area) continue;
     // Invariant: Evacuation candidates are just created when marking is
     // started. This means that sweeping has finished. Furthermore, at the end
     // of a GC all evacuation candidates are cleared and their slot buffers are
     // released.
     CHECK(!p->IsEvacuationCandidate());
-    CHECK_NULL(p->old_to_old_slots());
-    CHECK_NULL(p->typed_old_to_old_slots());
+    CHECK_NULL(p->slot_set<OLD_TO_OLD>());
+    CHECK_NULL(p->typed_slot_set<OLD_TO_OLD>());
     CHECK(p->SweepingDone());
     DCHECK(p->area_size() == area_size);
     pages.push_back(std::make_pair(p->LiveBytesFromFreeList(), p));
   }
 
   int candidate_count = 0;
   size_t total_live_bytes = 0;
 
   const bool reduce_memory = heap()->ShouldReduceMemory();
   if (FLAG_manual_evacuation_candidates_selection) {
@@ skipping 2651 matching lines @@
         return map_word.ToForwardingAddress();
       }
     }
     return object;
   }
 };
 
 MarkCompactCollector::Sweeper::ClearOldToNewSlotsMode
 MarkCompactCollector::Sweeper::GetClearOldToNewSlotsMode(Page* p) {
   AllocationSpace identity = p->owner()->identity();
-  if (p->old_to_new_slots() &&
+  if (p->slot_set<OLD_TO_NEW>() &&
       (identity == OLD_SPACE || identity == MAP_SPACE)) {
     return MarkCompactCollector::Sweeper::CLEAR_REGULAR_SLOTS;
-  } else if (p->typed_old_to_new_slots() && identity == CODE_SPACE) {
+  } else if (p->typed_slot_set<OLD_TO_NEW>() && identity == CODE_SPACE) {
     return MarkCompactCollector::Sweeper::CLEAR_TYPED_SLOTS;
   }
   return MarkCompactCollector::Sweeper::DO_NOT_CLEAR;
 }
 
 int MarkCompactCollector::Sweeper::RawSweep(
     Page* p, FreeListRebuildingMode free_list_mode,
     FreeSpaceTreatmentMode free_space_mode) {
   Space* space = p->owner();
   DCHECK_NOT_NULL(space);
@@ skipping 93 matching lines @@
                                              SlotSet::KEEP_EMPTY_BUCKETS);
     } else if (slots_clearing_mode == CLEAR_TYPED_SLOTS) {
       free_ranges.insert(std::pair<uint32_t, uint32_t>(
           static_cast<uint32_t>(free_start - p->address()),
           static_cast<uint32_t>(p->area_end() - p->address())));
     }
   }
 
   // Clear invalid typed slots after collection all free ranges.
   if (slots_clearing_mode == CLEAR_TYPED_SLOTS) {
-    p->typed_old_to_new_slots()->RemoveInvaldSlots(free_ranges);
+    p->typed_slot_set<OLD_TO_NEW>()->RemoveInvaldSlots(free_ranges);
   }
 
   // Clear the mark bits of that page and reset live bytes count.
   p->ClearLiveness();
 
   p->concurrent_sweeping_state().SetValue(Page::kSweepingDone);
   if (free_list_mode == IGNORE_FREE_LIST) return 0;
   return static_cast<int>(FreeList::GuaranteedAllocatable(max_freed_bytes));
 }
 
@@ skipping 34 matching lines @@
                                             IterationMode mode) {
   LiveObjectIterator<kBlackObjects> it(page);
   HeapObject* object = nullptr;
   while ((object = it.Next()) != nullptr) {
     DCHECK(ObjectMarking::IsBlack(object));
     if (!visitor->Visit(object)) {
       if (mode == kClearMarkbits) {
         page->markbits()->ClearRange(
             page->AddressToMarkbitIndex(page->area_start()),
             page->AddressToMarkbitIndex(object->address()));
-        if (page->old_to_new_slots() != nullptr) {
-          page->old_to_new_slots()->RemoveRange(
+        if (page->slot_set<OLD_TO_NEW>() != nullptr) {

[ulan, 2017/03/20 14:27:14]

Seems like potential race condition here. Should we first cache the value before
checking for null?

[Hannes Payer (out of office), 2017/03/20 15:07:08]

This site is safe since we never go from allocated to something else allocated.
However, I changed it to be bullet proof.

+          page->slot_set<OLD_TO_NEW>()->RemoveRange(
               0, static_cast<int>(object->address() - page->address()),
               SlotSet::PREFREE_EMPTY_BUCKETS);
         }
-        if (page->typed_old_to_new_slots() != nullptr) {
+        if (page->typed_slot_set<OLD_TO_NEW>() != nullptr) {
           RememberedSet<OLD_TO_NEW>::RemoveRangeTyped(page, page->address(),
                                                       object->address());
         }
         RecomputeLiveBytes(page);
       }
       return false;
     }
   }
   if (mode == kClearMarkbits) {
     page->ClearLiveness();
@@ skipping 82 matching lines @@
     EvacuateEpilogue();
   }
 
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap && !sweeper().sweeping_in_progress()) {
     VerifyEvacuation(heap());
   }
 #endif
 }
 
-template <PointerDirection direction>
+template <RememberedSetType type>
 class PointerUpdateJobTraits {
  public:
   typedef int PerPageData;  // Per page data is not used in this job.
   typedef int PerTaskData;  // Per task data is not used in this job.
 
   static bool ProcessPageInParallel(Heap* heap, PerTaskData, MemoryChunk* chunk,
                                     PerPageData) {
     UpdateUntypedPointers(heap, chunk);
     UpdateTypedPointers(heap, chunk);
     return true;
   }
   static const bool NeedSequentialFinalization = false;
   static void FinalizePageSequentially(Heap*, MemoryChunk*, bool, PerPageData) {
   }
 
  private:
   static void UpdateUntypedPointers(Heap* heap, MemoryChunk* chunk) {
-    if (direction == OLD_TO_NEW) {
+    if (type == OLD_TO_NEW) {
       RememberedSet<OLD_TO_NEW>::Iterate(chunk, [heap](Address slot) {
         return CheckAndUpdateOldToNewSlot(heap, slot);
       });
     } else {
       RememberedSet<OLD_TO_OLD>::Iterate(chunk, [](Address slot) {
         return UpdateSlot(reinterpret_cast<Object**>(slot));
       });
     }
   }
 
   static void UpdateTypedPointers(Heap* heap, MemoryChunk* chunk) {
-    if (direction == OLD_TO_OLD) {
+    if (type == OLD_TO_OLD) {
       Isolate* isolate = heap->isolate();
       RememberedSet<OLD_TO_OLD>::IterateTyped(
-          chunk, [isolate](SlotType type, Address host_addr, Address slot) {
-            return UpdateTypedSlotHelper::UpdateTypedSlot(isolate, type, slot,
-                                                          UpdateSlot);
+          chunk,
+          [isolate](SlotType slot_type, Address host_addr, Address slot) {
+            return UpdateTypedSlotHelper::UpdateTypedSlot(isolate, slot_type,
+                                                          slot, UpdateSlot);
           });
     } else {
       Isolate* isolate = heap->isolate();
       RememberedSet<OLD_TO_NEW>::IterateTyped(
           chunk,
-          [isolate, heap](SlotType type, Address host_addr, Address slot) {
+          [isolate, heap](SlotType slot_type, Address host_addr, Address slot) {
             return UpdateTypedSlotHelper::UpdateTypedSlot(
-                isolate, type, slot, [heap](Object** slot) {
+                isolate, slot_type, slot, [heap](Object** slot) {
                   return CheckAndUpdateOldToNewSlot(
                       heap, reinterpret_cast<Address>(slot));
                 });
           });
     }
   }
 
   static SlotCallbackResult CheckAndUpdateOldToNewSlot(Heap* heap,
                                                        Address slot_address) {
     // There may be concurrent action on slots in dead objects. Concurrent
@@ skipping 44 matching lines @@
 
 int NumberOfPointerUpdateTasks(int pages) {
   if (!FLAG_parallel_pointer_update) return 1;
   const int available_cores = Max(
       1, static_cast<int>(
              V8::GetCurrentPlatform()->NumberOfAvailableBackgroundThreads()));
   const int kPagesPerTask = 4;
   return Min(available_cores, (pages + kPagesPerTask - 1) / kPagesPerTask);
 }
 
-template <PointerDirection direction>
+template <RememberedSetType type>
 void UpdatePointersInParallel(Heap* heap, base::Semaphore* semaphore) {
-  PageParallelJob<PointerUpdateJobTraits<direction> > job(
+  PageParallelJob<PointerUpdateJobTraits<type> > job(
       heap, heap->isolate()->cancelable_task_manager(), semaphore);
-  RememberedSet<direction>::IterateMemoryChunks(
+  RememberedSet<type>::IterateMemoryChunks(
       heap, [&job](MemoryChunk* chunk) { job.AddPage(chunk, 0); });
   int num_pages = job.NumberOfPages();
   int num_tasks = NumberOfPointerUpdateTasks(num_pages);
   job.Run(num_tasks, [](int i) { return 0; });
 }
 
 class ToSpacePointerUpdateJobTraits {
  public:
   typedef std::pair<Address, Address> PerPageData;
   typedef PointersUpdatingVisitor* PerTaskData;
@@ skipping 134 matching lines @@
     const Sweeper::FreeSpaceTreatmentMode free_space_mode =
         Heap::ShouldZapGarbage() ? ZAP_FREE_SPACE : IGNORE_FREE_SPACE;
     if (identity == NEW_SPACE) {
       RawSweep(page, IGNORE_FREE_LIST, free_space_mode);
     } else {
       max_freed = RawSweep(page, REBUILD_FREE_LIST, free_space_mode);
     }
     DCHECK(page->SweepingDone());
 
     // After finishing sweeping of a page we clean up its remembered set.
-    if (page->typed_old_to_new_slots()) {
-      page->typed_old_to_new_slots()->FreeToBeFreedChunks();
+    if (page->typed_slot_set<OLD_TO_NEW>()) {

[ulan, 2017/03/20 14:27:15]

Cache the value before checking here and below?

[Hannes Payer (out of office), 2017/03/20 15:07:08]

As before, that should be safe. But let's be double safe.

+      page->typed_slot_set<OLD_TO_NEW>()->FreeToBeFreedChunks();
     }
-    if (page->old_to_new_slots()) {
-      page->old_to_new_slots()->FreeToBeFreedBuckets();
+    if (page->slot_set<OLD_TO_NEW>()) {
+      page->slot_set<OLD_TO_NEW>()->FreeToBeFreedBuckets();
     }
   }
 
   {
     base::LockGuard<base::Mutex> guard(&mutex_);
     swept_list_[identity].Add(page);
   }
   return max_freed;
 }
 
@@ skipping 144 matching lines @@
       // The target is always in old space, we don't have to record the slot in
       // the old-to-new remembered set.
       DCHECK(!heap()->InNewSpace(target));
       RecordRelocSlot(host, &rinfo, target);
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8