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..4b31cb73d8a62790415a36ceb575445b5d8dddff 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);

[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);
       }
@@ -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
@@ -595,13 +600,16 @@ void StoreBuffer::IteratePointersInStoreBuffer(
       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));
         }