- Do not bump allocate in old-space pages. Always use

vm/pages.h

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #ifndef VM_PAGES_H_
 #define VM_PAGES_H_
 
 #include <map>
 
 #include "vm/freelist.h"
@@ skipping 13 matching lines @@
   HeapPage* next() const { return next_; }
   void set_next(HeapPage* next) { next_ = next; }
 
   bool Contains(uword addr) {
     return memory_->Contains(addr);
   }
 
   uword start() const { return reinterpret_cast<uword>(this); }
   uword end() const { return memory_->end(); }
 
-  uword top() const { return top_; }
-  void set_top(uword top) { top_ = top; }
-
   uword first_object_start() const {
     return (reinterpret_cast<uword>(this) + sizeof(HeapPage));
   }
 
   void set_used(uword used) { used_ = used; }
   uword used() const { return used_; }
   void AddUsed(uword size) {
     used_ += size;
   }
 
-  uword TryBumpAllocate(intptr_t size) {
-    uword result = top();
-    intptr_t remaining_space = end() - result;
-    if (remaining_space < size) {
-      return 0;
-    }
-    set_top(result + size);
-    return result;
-  }
-
   void VisitObjects(ObjectVisitor* visitor) const;
   void VisitObjectPointers(ObjectPointerVisitor* visitor) const;
 
   RawObject* FindObject(FindObjectVisitor* visitor) const;
 
   void WriteProtect(bool read_only);
 
  private:
   static HeapPage* Initialize(VirtualMemory* memory, bool is_executable);
   static HeapPage* Allocate(intptr_t size, bool is_executable);
 
   // Deallocate the virtual memory backing this page. The page pointer to this
   // page becomes immediately inaccessible.
   void Deallocate();
 
   VirtualMemory* memory_;
   HeapPage* next_;
   uword used_;
-  uword top_;
+  uword alignment_;  // Needed to allocate old objects with kOldObjectAlignment.

[cshapiro, 2012/10/17 02:53:22]

This is a bit confusing.  Is this member referenced anywhere or is it just
padding?  It looks new, but I do not see anything that references it elsewhere
in this change.

If this is just padding, a compile time assert should check the alignment. 
Renaming this to padding_ would be more descriptive as well.

An alternative is to round up in first_object_start and do away with this member
all together.  That would seem to be more robust and would make a nice location
for a static or dynamic assert.

[Ivan Posva, 2012/10/18 17:34:21]

It is padding, which I tried to convey with the name alignment_.
I will reuse this slot for the end of the last object which corresponds to
memory_->end() for regularly sized pages and end of the object for large pages.
I have added a dynamic assert in allocation to ensure that all newly allocated
objects in PageSpace do have the required alignment.

 
   friend class PageSpace;
 
   DISALLOW_ALLOCATION();
   DISALLOW_IMPLICIT_CONSTRUCTORS(HeapPage);
 };
 
 
 // The history holds the timing information of the last garbage collection
 // runs.
@@ skipping 123 matching lines @@
 
   void* GetPeer(RawObject* raw_obj);
 
   int64_t PeerCount() const;
 
   PeerTable* GetPeerTable() { return &peer_table_; }
 
  private:
   static const intptr_t kAllocatablePageSize = kPageSize - sizeof(HeapPage);
 
-  void AllocatePage();
+  HeapPage* AllocatePage();
   void FreePage(HeapPage* page, HeapPage* previous_page);
   HeapPage* AllocateLargePage(intptr_t size);
   void FreeLargePage(HeapPage* page, HeapPage* previous_page);
   void FreePages(HeapPage* pages);
 
   static intptr_t LargePageSizeFor(intptr_t size);
 
   bool CanIncreaseCapacity(intptr_t increase) {
     ASSERT(capacity_ <= max_capacity_);
     return increase <= (max_capacity_ - capacity_);
   }
 
-  uword TryBumpAllocate(intptr_t size);
-
   FreeList freelist_;
 
   Heap* heap_;
 
   HeapPage* pages_;
   HeapPage* pages_tail_;
   HeapPage* large_pages_;
 
   PeerTable peer_table_;
 
-  // Page being used for bump allocation.
-  // The value has different meanings:
-  // NULL: Still bump allocating from last allocated fresh page.
-  // !NULL: Last page that had enough room to bump allocate, when we reach the
-  // tail page, we give up bump allocating.
-  HeapPage* bump_page_;
-
   // Various sizes being tracked for this generation.
   intptr_t max_capacity_;
   intptr_t capacity_;
   intptr_t in_use_;
 
   // Old-gen GC cycle count.
   int count_;
 
   bool is_executable_;
 
   // Keep track whether a MarkSweep is currently running.
   bool sweeping_;
 
   PageSpaceController page_space_controller_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(PageSpace);
 };
 
 }  // namespace dart
 
 #endif  // VM_PAGES_H_