Fix logic for doing incremental marking steps on tenured allocation.

src/heap/spaces.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 "src/v8.h"
 
 #include "src/base/bits.h"
 #include "src/base/platform/platform.h"
 #include "src/full-codegen.h"
 #include "src/heap/mark-compact.h"
@@ skipping 2340 matching lines @@
       page = Page::FromAddress(node->address());
       page->add_available_in_large_free_list(-(*node_size));
     }
   }
 
   DCHECK(IsVeryLong() || available() == SumFreeLists());
   return node;
 }
 
 
+void PagedSpace::SetTopAndLimit(Address top, Address limit) {
+  DCHECK(top == limit ||
+         Page::FromAddress(top) == Page::FromAddress(limit - 1));
+  MemoryChunk::UpdateHighWaterMark(allocation_info_.top());
+  allocation_info_.set_top(top);
+  allocation_info_.set_limit(limit);
+}
+
+
 // Allocation on the old space free list.  If it succeeds then a new linear
 // allocation space has been set up with the top and limit of the space.  If
 // the allocation fails then NULL is returned, and the caller can perform a GC
 // or allocate a new page before retrying.
 HeapObject* FreeList::Allocate(int size_in_bytes) {
   DCHECK(0 < size_in_bytes);
   DCHECK(size_in_bytes <= kMaxBlockSize);
   DCHECK(IsAligned(size_in_bytes, kPointerSize));
   // Don't free list allocate if there is linear space available.
   DCHECK(owner_->limit() - owner_->top() < size_in_bytes);
 
   int old_linear_size = static_cast<int>(owner_->limit() - owner_->top());
   // Mark the old linear allocation area with a free space map so it can be
   // skipped when scanning the heap.  This also puts it back in the free list
   // if it is big enough.
   owner_->Free(owner_->top(), old_linear_size);
 
-  owner_->heap()->incremental_marking()->OldSpaceStep(size_in_bytes -
-                                                      old_linear_size);

[Erik Corry Chromium.org, 2015/03/31 13:11:32]

This was basically wrong, basing the step on the size of one allocation plus
what was left of the old linear allocation area, but ignoring the rest of the
new linear allocation area.

-
   int new_node_size = 0;
   FreeSpace* new_node = FindNodeFor(size_in_bytes, &new_node_size);
   if (new_node == NULL) {
     owner_->SetTopAndLimit(NULL, NULL);
     return NULL;
   }
 
   int bytes_left = new_node_size - size_in_bytes;
   DCHECK(bytes_left >= 0);
 
 #ifdef DEBUG
   for (int i = 0; i < size_in_bytes / kPointerSize; i++) {
     reinterpret_cast<Object**>(new_node->address())[i] =
         Smi::FromInt(kCodeZapValue);
   }
 #endif
 
   // The old-space-step might have finished sweeping and restarted marking.
   // Verify that it did not turn the page of the new node into an evacuation
   // candidate.
   DCHECK(!MarkCompactCollector::IsOnEvacuationCandidate(new_node));
 
-  const int kThreshold = IncrementalMarking::kAllocatedThreshold;
+  // An old-space step will mark more data per byte allocated, because old space
+  // allocation is more serious.  We don't want the pause to be bigger, so we
+  // do marking after a smaller amount of allocation.
+  const int kThreshold = IncrementalMarking::kAllocatedThreshold *
+                         IncrementalMarking::kOldSpaceAllocationMarkingFactor;
 
   // Memory in the linear allocation area is counted as allocated.  We may free
   // a little of this again immediately - see below.
   owner_->Allocate(new_node_size);
 
   if (owner_->heap()->inline_allocation_disabled()) {
     // Keep the linear allocation area empty if requested to do so, just
     // return area back to the free list instead.
     owner_->Free(new_node->address() + size_in_bytes, bytes_left);
     DCHECK(owner_->top() == NULL && owner_->limit() == NULL);
   } else if (bytes_left > kThreshold &&
-             owner_->heap()->incremental_marking()->IsMarkingIncomplete() &&
-             FLAG_incremental_marking_steps) {
+             owner_->heap()->incremental_marking()->CanDoSteps()) {
     int linear_size = owner_->RoundSizeDownToObjectAlignment(kThreshold);
+
     // We don't want to give too large linear areas to the allocator while
     // incremental marking is going on, because we won't check again whether
     // we want to do another increment until the linear area is used up.
     owner_->Free(new_node->address() + size_in_bytes + linear_size,
                  new_node_size - size_in_bytes - linear_size);
     owner_->SetTopAndLimit(new_node->address() + size_in_bytes,
                            new_node->address() + size_in_bytes + linear_size);
-  } else if (bytes_left > 0) {
-    // Normally we give the rest of the node to the allocator as its new
-    // linear allocation area.
-    owner_->SetTopAndLimit(new_node->address() + size_in_bytes,
-                           new_node->address() + new_node_size);
+    owner_->heap()->incremental_marking()->OldSpaceStep(size_in_bytes +
+                                                        linear_size);
   } else {
-    // TODO(gc) Try not freeing linear allocation region when bytes_left
-    // are zero.
-    owner_->SetTopAndLimit(NULL, NULL);
+    if (owner_->heap()->incremental_marking()->CanDoSteps()) {
+      owner_->heap()->incremental_marking()->OldSpaceStep(new_node_size);
+    } else if (new_node_size > kThreshold) {
+      // When we give big chunks to the old space allocator we do a little
+      // step in case the incremental marker wants to start.
+      owner_->heap()->incremental_marking()->OldSpaceStep(kThreshold);
+    }
+    if (bytes_left > 0) {
+      // Normally we give the rest of the node to the allocator as its new
+      // linear allocation area.
+      owner_->SetTopAndLimit(new_node->address() + size_in_bytes,
+                             new_node->address() + new_node_size);
+    } else {
+      // TODO(gc) Try not freeing linear allocation region when bytes_left
+      // are zero.
+      owner_->SetTopAndLimit(NULL, NULL);
+    }
   }
 
   return new_node;
 }
 
 
 intptr_t FreeList::EvictFreeListItems(Page* p) {
   intptr_t sum = huge_list_.EvictFreeListItemsInList(p);
   p->set_available_in_huge_free_list(0);
 
@@ skipping 696 matching lines @@
       mark_size += heap()->GcSafeSizeOfOldObjectFunction()(object);
     }
     object->ShortPrint();
     PrintF("\n");
   }
   printf(" --------------------------------------\n");
   printf(" Marked: %x, LiveCount: %x\n", mark_size, LiveBytes());
 }
 
 #endif  // DEBUG
+
 }
 }  // namespace v8::internal