Synchronize store buffer processing and concurrent sweeping.

src/store-buffer.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 337 matching lines @@
 }
 
 
 void StoreBuffer::GCPrologue() {
   ClearFilteringHashSets();
   during_gc_ = true;
 }
 
 
 #ifdef VERIFY_HEAP
-static void DummyScavengePointer(HeapObject** p, HeapObject* o) {
+static void DummyScavengePointer(HeapObject** p, HeapObject* o, Object* old) {
   // Do nothing.
 }
 
 
 void StoreBuffer::VerifyPointers(PagedSpace* space,
                                  RegionCallback region_callback) {
   PageIterator it(space);
 
   while (it.has_next()) {
     Page* page = it.next();
@@ skipping 51 matching lines @@
 
 void StoreBuffer::FindPointersToNewSpaceInRegion(
     Address start,
     Address end,
     ObjectSlotCallback slot_callback,
     bool clear_maps) {
   for (Address slot_address = start;
        slot_address < end;
        slot_address += kPointerSize) {
     Object** slot = reinterpret_cast<Object**>(slot_address);
-    if (heap_->InNewSpace(*slot)) {
+    Object* old_value = *slot;
+    if (heap_->InNewSpace(old_value)) {
       HeapObject* object = reinterpret_cast<HeapObject*>(*slot);

[Jarin, 2014/04/08 13:00:54]

Why could not we get rid of the read of *slot here and use old_value instead?

[Hannes Payer (out of office), 2014/04/08 14:04:35]

Yes, this one should not be there. I did not upload the latest version. All slot
accessors are going through annotated load/stores.

       ASSERT(object->IsHeapObject());
       // The new space object was not promoted if it still contains a map
       // pointer. Clear the map field now lazily.
       if (clear_maps) ClearDeadObject(object);
-      slot_callback(reinterpret_cast<HeapObject**>(slot), object);
+      slot_callback(reinterpret_cast<HeapObject**>(slot), object, old_value);

[Jarin, 2014/04/08 13:00:54]

This extra argument should not be necessary - we could make object = old_value
here.

[Hannes Payer (out of office), 2014/04/08 14:04:35]

Done.
Good point, I was too conservative here. 

       if (heap_->InNewSpace(*slot)) {
         EnterDirectlyIntoStoreBuffer(slot_address);
       }
     }
   }
 }
 
 
 // Compute start address of the first map following given addr.
 static inline Address MapStartAlign(Address addr) {
@@ skipping 77 matching lines @@
   Address visitable_start = page->area_start();
   Address end_of_page = page->area_end();
 
   Address visitable_end = visitable_start;
 
   Object* free_space_map = heap_->free_space_map();
   Object* two_pointer_filler_map = heap_->two_pointer_filler_map();
   Object* constant_pool_array_map = heap_->constant_pool_array_map();
 
   while (visitable_end < end_of_page) {
-    Object* o = *reinterpret_cast<Object**>(visitable_end);
+    // The sweeper thread concurrently may write free space maps and size to
+    // this page. We need acquire load here to make sure that we get a
+    // consistent view of maps and their sizes.
+    Object* o = reinterpret_cast<Object*>(
+        Acquire_Load(reinterpret_cast<AtomicWord*>(visitable_end)));
     // Skip fillers or constant pool arrays (which never contain new-space
     // pointers but can contain pointers which can be confused for fillers)
     // but not things that look like fillers in the special garbage section
     // which can contain anything.
     if (o == free_space_map ||
         o == two_pointer_filler_map ||
         o == constant_pool_array_map ||
         (visitable_end == space->top() && visitable_end != space->limit())) {
       if (visitable_start != visitable_end) {
         // After calling this the special garbage section may have moved.
@@ skipping 43 matching lines @@
     bool clear_maps) {
   Address* limit = old_top_;
   old_top_ = old_start_;
   {
     DontMoveStoreBufferEntriesScope scope(this);
     for (Address* current = old_start_; current < limit; current++) {
 #ifdef DEBUG
       Address* saved_top = old_top_;
 #endif
       Object** slot = reinterpret_cast<Object**>(*current);
-      Object* object = *slot;
+      Object* object = reinterpret_cast<Object*>(
+          NoBarrier_Load(reinterpret_cast<AtomicWord*>(slot)));
       if (heap_->InFromSpace(object)) {
         HeapObject* heap_object = reinterpret_cast<HeapObject*>(object);
         // The new space object was not promoted if it still contains a map
         // pointer. Clear the map field now lazily.
         if (clear_maps) ClearDeadObject(heap_object);
-        slot_callback(reinterpret_cast<HeapObject**>(slot), heap_object);
+        slot_callback(reinterpret_cast<HeapObject**>(slot),
+                      heap_object,
+                      object);
         if (heap_->InNewSpace(*slot)) {
           EnterDirectlyIntoStoreBuffer(reinterpret_cast<Address>(slot));
         }
       }
       ASSERT(old_top_ == saved_top + 1 || old_top_ == saved_top);
     }
   }
 }
 
 
@@ skipping 116 matching lines @@
     }
     old_buffer_is_sorted_ = false;
     old_buffer_is_filtered_ = false;
     *old_top_++ = reinterpret_cast<Address>(int_addr << kPointerSizeLog2);
     ASSERT(old_top_ <= old_limit_);
   }
   heap_->isolate()->counters()->store_buffer_compactions()->Increment();
 }
 
 } }  // namespace v8::internal