Remove unnecessary NoBarrier_load instructions in store buffer.

src/heap/store-buffer.cc

 // Copyright 2011 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 <algorithm>
 
 #include "src/v8.h"
 
 #include "src/base/atomicops.h"
 #include "src/counters.h"
@@ skipping 332 matching lines @@
   for (HeapObject* object = it.Next(); object != NULL; object = it.Next()) {
     if (object->IsFixedArray()) {
       Address slot_address = object->address();
       Address end = object->address() + object->Size();
 
       while (slot_address < end) {
         HeapObject** slot = reinterpret_cast<HeapObject**>(slot_address);
         // When we are not in GC the Heap::InNewSpace() predicate
         // checks that pointers which satisfy predicate point into
         // the active semispace.
-        Object* object = reinterpret_cast<Object*>(
-            base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
+        Object* object = *slot;
         heap_->InNewSpace(object);
         slot_address += kPointerSize;
       }
     }
   }
 }
 #endif
 
 
 void StoreBuffer::Verify() {
@@ skipping 10 matching lines @@
     Verify();
   }
 #endif
 }
 
 
 void StoreBuffer::ProcessOldToNewSlot(Address slot_address,
                                       ObjectSlotCallback slot_callback,
                                       bool clear_maps) {
   Object** slot = reinterpret_cast<Object**>(slot_address);
-  Object* object = reinterpret_cast<Object*>(
-      base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
+  Object* object = *slot;
 
   // If the object is not in from space, it must be a duplicate store buffer
   // entry and the slot was already updated.
   if (heap_->InFromSpace(object)) {
     HeapObject* heap_object = reinterpret_cast<HeapObject*>(object);
     DCHECK(heap_object->IsHeapObject());
     // The new space object was not promoted if it still contains a map
     // pointer. Clear the map field now lazily (during full GC).
     if (clear_maps) ClearDeadObject(heap_object);
     slot_callback(reinterpret_cast<HeapObject**>(slot), heap_object);
-    object = reinterpret_cast<Object*>(
-        base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
+    object = *slot;
     // If the object was in from space before and is after executing the
     // callback in to space, the object is still live.
     // Unfortunately, we do not know about the slot. It could be in a
     // just freed free space object.
     if (heap_->InToSpace(object)) {
       EnterDirectlyIntoStoreBuffer(reinterpret_cast<Address>(slot));
     }
   }
 }
 
@@ skipping 24 matching lines @@
   }
 }
 
 
 void StoreBuffer::ClearInvalidStoreBufferEntries() {
   Compact();
   Address* new_top = old_start_;
   for (Address* current = old_start_; current < old_top_; current++) {
     Address addr = *current;
     Object** slot = reinterpret_cast<Object**>(*current);
+    // Use a NoBarrier_Load here since the slot can be in a dead object
+    // which may be touched by the concurrent sweeper thread.

[Jarin, 2015/03/10 13:32:25]

Why did not you update this one?

[Hannes Payer (out of office), 2015/03/10 13:51:17]

Because the stale pointers may be accessed concurrently by the sweeper thread.
After executing this method we have valid pointers in the store buffer and don't
have to worry about the sweeper thread anymore.

     Object* object = reinterpret_cast<Object*>(
         base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
     if (heap_->InNewSpace(object)) {
       if (heap_->mark_compact_collector()->IsSlotInLiveObject(
               reinterpret_cast<HeapObject**>(slot),
               reinterpret_cast<HeapObject*>(object))) {
         *new_top++ = addr;
       }
     }
   }
   old_top_ = new_top;
   ClearFilteringHashSets();
 
   // Don't scan on scavenge dead large objects.
   LargeObjectIterator it(heap_->lo_space());
   for (HeapObject* object = it.Next(); object != NULL; object = it.Next()) {
     MemoryChunk* chunk = MemoryChunk::FromAddress(object->address());
     if (chunk->scan_on_scavenge() && !Marking::MarkBitFrom(object).Get()) {
       chunk->set_scan_on_scavenge(false);
     }
   }
 }
 
 
 void StoreBuffer::VerifyValidStoreBufferEntries() {
   for (Address* current = old_start_; current < old_top_; current++) {
     Object** slot = reinterpret_cast<Object**>(*current);
-    Object* object = reinterpret_cast<Object*>(
-        base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
+    Object* object = *slot;
     CHECK(heap_->InNewSpace(object));
     heap_->mark_compact_collector()->VerifyIsSlotInLiveObject(
         reinterpret_cast<HeapObject**>(slot),
         reinterpret_cast<HeapObject*>(object));
   }
 }
 
 
 void StoreBuffer::IteratePointersToNewSpace(ObjectSlotCallback slot_callback) {
   IteratePointersToNewSpace(slot_callback, false);
@@ skipping 173 matching lines @@
     }
     old_buffer_is_sorted_ = false;
     old_buffer_is_filtered_ = false;
     *old_top_++ = reinterpret_cast<Address>(int_addr << kPointerSizeLog2);
     DCHECK(old_top_ <= old_limit_);
   }
   heap_->isolate()->counters()->store_buffer_compactions()->Increment();
 }
 }
 }  // namespace v8::internal