Synchronize store buffer processing and concurrent sweeping.

src/store-buffer.cc

Index: src/store-buffer.cc
diff --git a/src/store-buffer.cc b/src/store-buffer.cc
index a1479b2b9aaaac37c8bf6a8fa251f592b6fd33a7..82edc2bffe5b85561acfa145064c82a03f3b96c9 100644
--- a/src/store-buffer.cc
+++ b/src/store-buffer.cc
@@ -355,7 +355,7 @@ void StoreBuffer::GCPrologue() {
 
 
 #ifdef VERIFY_HEAP
-static void DummyScavengePointer(HeapObject** p, HeapObject* o) {
+static void DummyScavengePointer(HeapObject** p, HeapObject* o, Object* old) {
   // Do nothing.
 }
 
@@ -427,13 +427,14 @@ void StoreBuffer::FindPointersToNewSpaceInRegion(
        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);
       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);
       if (heap_->InNewSpace(*slot)) {
         EnterDirectlyIntoStoreBuffer(slot_address);
       }
@@ -531,7 +532,11 @@ void StoreBuffer::FindPointersToNewSpaceOnPage(
   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
@@ -594,14 +599,18 @@ void StoreBuffer::IteratePointersInStoreBuffer(
 #ifdef DEBUG
       Address* saved_top = old_top_;
 #endif
-      Object** slot = reinterpret_cast<Object**>(*current);
-      Object* object = *slot;
+      Object** slot = reinterpret_cast<Object**>(
+          NoBarrier_Load(reinterpret_cast<AtomicWord*>(current)));

[Michael Starzinger, 2014/04/08 10:35:31]

I don't understand the reason for the atomic read of "current" here. Is it
really necessary? Otherwise let's drop it.

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

Done.

Nope, not necessary. The sweeper is not touching the store buffer. This change
slipped in somehow. I removed it.

+      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));
         }