Changes new space allocation from simple bump pointer allocation from

runtime/vm/heap.cc

Index: runtime/vm/heap.cc
diff --git a/runtime/vm/heap.cc b/runtime/vm/heap.cc
index 334e90364f5d6e16e70ef2a084ad47a76d0a5d78..7798113b28a1354593d489d8f4824ebc1827568b 100644
--- a/runtime/vm/heap.cc
+++ b/runtime/vm/heap.cc
@@ -57,21 +57,80 @@ Heap::~Heap() {
   }
 }
 
+void Heap::MakeTLABIterable(uword start, uword end) {

[rmacnak, 2017/07/25 21:02:44]

Consider recasting as Scavenger::AbandonRemainingTLAB(Thread* thread) for
symmetry with PageSpace::AbandonBumpAllocation.

This also looks like a good place to set Thread's top/end to indicate it does
not currently have a TLAB.

[danunez, 2017/07/26 20:19:20]

Good call.

+  ASSERT(end >= start);
+  intptr_t size = end - start;

[rmacnak, 2017/07/25 21:02:44]

Assert size is a multiple of kObjectAlignment

[danunez, 2017/07/26 20:19:20]

Done.

+  if (end == new_space_.end()) {
+    size = 0;
+  }
+  if (size > kObjectAlignment) {

[rmacnak, 2017/07/25 21:02:44]

Create a FreeListElement for size == kObjectAlignment too. The only special case
should be size == 0.

[danunez, 2017/07/26 20:19:20]

Done.

+    FreeListElement::AsElement(start, size);
+    ASSERT(RawObject::FromAddr(start)->Size() == size);
+    ASSERT(start + size == new_space_.top());
+  }
+}
+
+intptr_t Heap::CalculateTLABSize() {
+  intptr_t size = new_space_.end() - new_space_.top();
+  return Utils::RoundDown(size, kObjectAlignment);
+}
+
 uword Heap::AllocateNew(intptr_t size) {
   ASSERT(Thread::Current()->no_safepoint_scope_depth() == 0);
   // Currently, only the Dart thread may allocate in new space.
   isolate()->AssertCurrentThreadIsMutator();
   Thread* thread = Thread::Current();
   uword addr = new_space_.TryAllocateInTLAB(thread, size);
+
+  if (addr != 0) {
+    return addr;
+  }
+
+  intptr_t tlab_size = CalculateTLABSize();
+
+  if (tlab_size > 0 && size > tlab_size) {
+    return AllocateOld(size, HeapPage::kData);
+  }
+
+  MakeTLABIterable(thread->top(), thread->end());
+
+  if (tlab_size > 0) {
+    uword tlab_top = new_space_.TryAllocateNewTLAB(tlab_size);
+
+    if (tlab_top != 0) {
+      // Set TLS to tlab_top and tlab_top+tlab_size
+      thread->set_top(tlab_top);
+      thread->set_end(new_space_.top());

[rmacnak, 2017/07/25 21:02:44]

tlab_top + tlab_size

(Thinking forward, new_space_.top() may be out of date if another thread
allocates a TLAB)

[danunez, 2017/07/26 20:19:20]

That is a very good point, especially since that section would not be locked in
the future.

I will also add the following assertion

ASSERT(tlab_top + tlab_size == new_space_.top())

Addressing the future comment, I will push this setup into TryAllocateNewTLAB.

+      ASSERT(thread->top() < new_space_.top());
+      addr = new_space_.TryAllocateInTLAB(thread, size);
+      ASSERT(addr != 0);
+      return addr;
+    }
+  }
+
   if (addr == 0) {

[rmacnak, 2017/07/25 21:02:45]

addr must be 0 if we reach here, right?

[danunez, 2017/07/26 20:19:20]

Correct. Consider that removed.

     // This call to CollectGarbage might end up "reusing" a collection spawned
     // from a different thread and will be racing to allocate the requested
     // memory with other threads being released after the collection.
     CollectGarbage(kNew);
-    addr = new_space_.TryAllocateInTLAB(thread, size);
-    if (addr == 0) {
-      return AllocateOld(size, HeapPage::kData);
+
+    intptr_t tlab_size = CalculateTLABSize();
+    uword tlab_top = new_space_.TryAllocateNewTLAB(tlab_size);
+
+    if (tlab_top != 0) {
+      // Set TLS to tlab_top and tlab_top+tlab_size
+      thread->set_top(tlab_top);
+      thread->set_end(new_space_.top());

[rmacnak, 2017/07/25 21:02:44]

tlab_top + tlab_size

This thread setup seems like it should be pushed into TryAllocateNewTLAB instead
of repeated.

[danunez, 2017/07/26 20:19:20]

Done.

+      ASSERT(thread->top() < new_space_.top());
+      addr = new_space_.TryAllocateInTLAB(thread, size);
+      // It is possible a GC doesn't clear enough space.
+      // In that case, we must fail through and allocate into old space.

[rmacnak, 2017/07/25 21:02:44]

fall through

[danunez, 2017/07/26 20:19:20]

Done.

+      if (addr != 0) {
+        return addr;
+      }
     }
+
+    return AllocateOld(size, HeapPage::kData);
   }
   return addr;
 }
@@ -546,13 +605,14 @@ bool Heap::VerifyGC(MarkExpectation mark_expectation) const {
   StackZone stack_zone(Thread::Current());
 
   // Change the new space's top_ with the more up-to-date thread's view of top_
-  new_space_.FlushTLS();
+  uword top_bk = new_space_.FlushTLS();

[rmacnak, 2017/07/25 21:02:44]

Consider saved_top since saved_x is the commonly used name in our codebase.

[danunez, 2017/07/26 20:19:20]

Done.

 
   ObjectSet* allocated_set =
       CreateAllocatedObjectSet(stack_zone.GetZone(), mark_expectation);
   VerifyPointersVisitor visitor(isolate(), allocated_set);
   VisitObjectPointers(&visitor);
 
+  new_space_.UnflushTLS(top_bk);
   // Only returning a value so that Heap::Validate can be called from an ASSERT.
   return true;
 }