Make sure that we grow the slots buffer when we are in evacuation scope.

src/mark-compact.h

Index: src/mark-compact.h
diff --git a/src/mark-compact.h b/src/mark-compact.h
index 6019f6c649e7fd0aae35b1773869c50caec53c24..7d1b754b629c89057acca3247f4652e5ef700c22 100644
--- a/src/mark-compact.h
+++ b/src/mark-compact.h
@@ -256,9 +256,8 @@ class MarkingDeque {
 
 class SlotsBufferAllocator {
  public:
-  SlotsBuffer* AllocateBuffer(SlotsBuffer* next_buffer);
+  SlotsBuffer* AllocateBuffer(Heap* heap, SlotsBuffer* next_buffer);
   void DeallocateBuffer(SlotsBuffer* buffer);
-
   void DeallocateChain(SlotsBuffer** buffer_address);
 };
 
@@ -279,8 +278,8 @@ class SlotsBuffer {
  public:
   typedef Object** ObjectSlot;
 
-  explicit SlotsBuffer(SlotsBuffer* next_buffer)
-      : idx_(0), chain_length_(1), next_(next_buffer) {
+  SlotsBuffer(Heap* heap, SlotsBuffer* next_buffer)
+      : idx_(0), chain_length_(1), heap_(heap), next_(next_buffer) {
     if (next_ != NULL) {
       chain_length_ = next_->chain_length_ + 1;
     }
@@ -324,9 +323,9 @@ class SlotsBuffer {
     return "UNKNOWN SlotType";
   }
 
-  void UpdateSlots(Heap* heap);
+  void UpdateSlots();
 

[Michael Starzinger, 2014/03/06 10:47:25]

nit: These two should be made protected IMHO.

-  void UpdateSlotsWithFilter(Heap* heap);
+  void UpdateSlotsWithFilter();
 
   SlotsBuffer* next() { return next_; }
 
@@ -344,14 +343,13 @@ class SlotsBuffer {
     return idx_ < kNumberOfElements - 1;
   }
 
-  static void UpdateSlotsRecordedIn(Heap* heap,
-                                    SlotsBuffer* buffer,
-                                    bool code_slots_filtering_required) {
+  inline void UpdateSlotsRecordedIn(bool code_slots_filtering_required) {

[Michael Starzinger, 2014/03/06 10:47:25]

nit: The naming is a little bit weird now, because it no longer takes an
argument to which the "in" applies. Can we rename the method to
"UpdateRecordedSlots" instead?

+    SlotsBuffer* buffer = this;
     while (buffer != NULL) {
       if (code_slots_filtering_required) {
-        buffer->UpdateSlotsWithFilter(heap);
+        buffer->UpdateSlotsWithFilter();
       } else {
-        buffer->UpdateSlots(heap);
+        buffer->UpdateSlots();
       }
       buffer = buffer->next();
     }
@@ -366,38 +364,31 @@ class SlotsBuffer {
     return buffer != NULL && buffer->chain_length_ >= kChainLengthThreshold;
   }
 
-  INLINE(static bool AddTo(SlotsBufferAllocator* allocator,
+  INLINE(static bool AddTo(Heap* heap,
+                           SlotsBufferAllocator* allocator,
                            SlotsBuffer** buffer_address,
                            ObjectSlot slot,
-                           AdditionMode mode)) {
-    SlotsBuffer* buffer = *buffer_address;
-    if (buffer == NULL || buffer->IsFull()) {
-      if (mode == FAIL_ON_OVERFLOW && ChainLengthThresholdReached(buffer)) {
-        allocator->DeallocateChain(buffer_address);
-        return false;
-      }
-      buffer = allocator->AllocateBuffer(buffer);
-      *buffer_address = buffer;
-    }
-    buffer->Add(slot);
-    return true;
-  }
+                           AdditionMode mode));
 
-  static bool IsTypedSlot(ObjectSlot slot);
+  INLINE(static bool AddTo(Heap* heap,
+                           SlotsBufferAllocator* allocator,
+                           SlotsBuffer** buffer_address,
+                           SlotType type,
+                           Address addr,
+                           AdditionMode mode));
 
-  static bool AddTo(SlotsBufferAllocator* allocator,
-                    SlotsBuffer** buffer_address,
-                    SlotType type,
-                    Address addr,
-                    AdditionMode mode);
+  Heap* heap() const { return heap_; }
 
-  static const int kNumberOfElements = 1021;
+  static bool IsTypedSlot(ObjectSlot slot);
+
+  static const int kNumberOfElements = 1020;

[ulan, 2014/03/05 15:40:18]

Why is this change to 1020 necessary?

[Hannes Payer (out of office), 2014/03/05 20:39:29]

The slots buffer should fit on a page. After adding one word (heap pointer), it
would overflow the page.

 
  private:
   static const int kChainLengthThreshold = 15;
 
   intptr_t idx_;
   intptr_t chain_length_;
+  Heap* heap_;
   SlotsBuffer* next_;
   ObjectSlot slots_[kNumberOfElements];
 };
@@ -747,6 +738,20 @@ class MarkCompactCollector {
   // to artifically keep AllocationSites alive for a time.
   void MarkAllocationSite(AllocationSite* site);
 
+  void IncrementEvacuationScope() {
+    evacuation_scope_++;
+    ASSERT(evacuation_scope_ <= 1);

[Michael Starzinger, 2014/03/06 10:47:25]

These two ASSERTS combined basically enforce ...

ASSERT(evacuation_scope_ == 1);  // in IncrementEvacuationScope
ASSERT(evacuation_scope_ == 0);  // in DecrementEvacuationScope

Can we change it to this instead? That would be much clearer.

+  }
+
+  void DecrementEvacuationScope() {
+    evacuation_scope_--;
+    ASSERT(evacuation_scope_ >= 0);
+  }
+
+  bool IsInsideEvacuationScope() {
+    return evacuation_scope_ > 0;
+  }
+
  private:
   class SweeperTask;
 
@@ -974,6 +979,8 @@ class MarkCompactCollector {
   SmartPointer<FreeList> free_list_old_data_space_;
   SmartPointer<FreeList> free_list_old_pointer_space_;
 
+  int evacuation_scope_;
+
   friend class Heap;
 };
 
@@ -1038,6 +1045,21 @@ class SequentialSweepingScope BASE_EMBEDDED {
 };
 
 
+class EvacuationScope V8_FINAL {

[Michael Starzinger, 2014/03/06 10:47:25]

High-level idea: This scope pretty much coincides with "state_ == SWEEP_SPACES",
would it make sense to use the state variable instead, or do you think a
separate scope brings additional advantages?

+ public:
+  explicit EvacuationScope(MarkCompactCollector* mark_compact) :
+      mark_compact_(mark_compact) {
+    mark_compact_->IncrementEvacuationScope();
+  }
+  ~EvacuationScope() {
+    mark_compact_->DecrementEvacuationScope();
+  }
+
+ private:
+  MarkCompactCollector* mark_compact_;
+};
+
+
 const char* AllocationSpaceName(AllocationSpace space);
 
 } }  // namespace v8::internal