Revert "[heap] introduce SpaceWithLinearArea class"

This reverts commit 8dd405e3.

Reason for revert: MSAN errors: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/18598

Original change's description:
> [heap] introduce SpaceWithLinearArea class
> 
> NewSpace and OldSpace have linear allocation areas, but presently the
> implementation doesn't share any code and there are subtle differences.
> This CL introduces a superclass 'SpaceWithLinearArea' that will be used
> to refactor and share code.
> 
> Change-Id: I741e6a6ebb9e75c111287214fd1f555fba62c452
> Reviewed-on: https://chromium-review.googlesource.com/809504
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
> Cr-Commit-Position: refs/heads/master@{#49890}

TBR=ulan@chromium.org,ofrobots@google.com

Change-Id: Ibd325c2965b7b8dea36b34a8c4a3ee06ea24e515
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/810664Reviewed-by: Ali Ijaz Sheikh <ofrobots@google.com>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#49892}

src/heap/spaces.cc

@@ -1374,11 +1374,14 @@ intptr_t Space::GetNextInlineAllocationStepSize() {
 
 PagedSpace::PagedSpace(Heap* heap, AllocationSpace space,
                        Executability executable)
-    : SpaceWithLinearArea(heap, space, executable),
+    : Space(heap, space, executable),
       anchor_(this),
-      free_list_(this) {
+      free_list_(this),
+      top_on_previous_step_(0) {
   area_size_ = MemoryAllocator::PageAreaSize(space);
   accounting_stats_.Clear();
+
+  allocation_info_.Reset(nullptr, nullptr);
 }
 
 
@@ -1638,7 +1641,6 @@ Address PagedSpace::ComputeLimit(Address start, Address end,
   }
 }
 
-// TODO(ofrobots): refactor this code into SpaceWithLinearArea
 void PagedSpace::StartNextInlineAllocationStep() {
   if (!allocation_observers_paused_ && SupportsInlineAllocation()) {
     top_on_previous_step_ = allocation_observers_.empty() ? 0 : top();
@@ -2148,7 +2150,7 @@ bool NewSpace::EnsureAllocation(int size_in_bytes,
   return true;
 }
 
-// TODO(ofrobots): refactor this code into SpaceWithLinearArea
+
 void NewSpace::StartNextInlineAllocationStep() {
   if (!allocation_observers_paused_) {
     top_on_previous_step_ =
@@ -2157,13 +2159,26 @@ void NewSpace::StartNextInlineAllocationStep() {
   }
 }
 
-void SpaceWithLinearArea::AddAllocationObserver(AllocationObserver* observer) {
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void NewSpace::AddAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::AddAllocationObserver(observer);
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::AddAllocationObserver(AllocationObserver* observer) {
   InlineAllocationStep(top(), top(), nullptr, 0);
   Space::AddAllocationObserver(observer);
 }
 
-void SpaceWithLinearArea::RemoveAllocationObserver(
-    AllocationObserver* observer) {
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void NewSpace::RemoveAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::RemoveAllocationObserver(observer);
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::RemoveAllocationObserver(AllocationObserver* observer) {
   InlineAllocationStep(top(), top(), nullptr, 0);
   Space::RemoveAllocationObserver(observer);
 }
@@ -2178,22 +2193,43 @@ void NewSpace::PauseAllocationObservers() {
 
 void PagedSpace::PauseAllocationObservers() {
   // Do a step to account for memory allocated so far.
-  // TODO(ofrobots): Refactor into SpaceWithLinearArea. Note subtle difference
-  // from NewSpace version.
   InlineAllocationStep(top(), nullptr, nullptr, 0);
   Space::PauseAllocationObservers();
   top_on_previous_step_ = 0;
 }
 
-void SpaceWithLinearArea::ResumeAllocationObservers() {
+void NewSpace::ResumeAllocationObservers() {
+  DCHECK_NULL(top_on_previous_step_);
+  Space::ResumeAllocationObservers();
+  StartNextInlineAllocationStep();
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::ResumeAllocationObservers() {
   DCHECK_NULL(top_on_previous_step_);
   Space::ResumeAllocationObservers();
   StartNextInlineAllocationStep();
 }
 
-void SpaceWithLinearArea::InlineAllocationStep(Address top, Address new_top,
-                                               Address soon_object,
-                                               size_t size) {
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::InlineAllocationStep(Address top, Address new_top,
+                                      Address soon_object, size_t size) {
+  if (top_on_previous_step_) {
+    if (top < top_on_previous_step_) {
+      // Generated code decreased the top pointer to do folded allocations.
+      DCHECK_NOT_NULL(top);
+      DCHECK_EQ(Page::FromAllocationAreaAddress(top),
+                Page::FromAllocationAreaAddress(top_on_previous_step_));
+      top_on_previous_step_ = top;
+    }
+    int bytes_allocated = static_cast<int>(top - top_on_previous_step_);
+    AllocationStep(bytes_allocated, soon_object, static_cast<int>(size));
+    top_on_previous_step_ = new_top;
+  }
+}
+
+void NewSpace::InlineAllocationStep(Address top, Address new_top,
+                                    Address soon_object, size_t size) {
   if (top_on_previous_step_) {
     if (top < top_on_previous_step_) {
       // Generated code decreased the top pointer to do folded allocations.

src/heap/spaces.h

@@ -1966,50 +1966,7 @@ class LocalAllocationBuffer {
   AllocationInfo allocation_info_;
 };
 
-class SpaceWithLinearArea : public Space {
- public:
-  SpaceWithLinearArea(Heap* heap, AllocationSpace id, Executability executable)
-      : Space(heap, id, executable), top_on_previous_step_(0) {
-    allocation_info_.Reset(nullptr, nullptr);
-  }
-
-  // Returns the allocation pointer in this space.
-  Address top() { return allocation_info_.top(); }
-  Address limit() { return allocation_info_.limit(); }
-
-  // The allocation top address.
-  Address* allocation_top_address() { return allocation_info_.top_address(); }
-
-  // The allocation limit address.
-  Address* allocation_limit_address() {
-    return allocation_info_.limit_address();
-  }
-
-  // If we are doing inline allocation in steps, this method performs the 'step'
-  // operation. top is the memory address of the bump pointer at the last
-  // inline allocation (i.e. it determines the numbers of bytes actually
-  // allocated since the last step.) new_top is the address of the bump pointer
-  // where the next byte is going to be allocated from. top and new_top may be
-  // different when we cross a page boundary or reset the space.
-  // TODO(ofrobots): clarify the precise difference between this and
-  // Space::AllocationStep.
-  void InlineAllocationStep(Address top, Address new_top, Address soon_object,
-                            size_t size);
-
-  V8_EXPORT_PRIVATE void AddAllocationObserver(
-      AllocationObserver* observer) override;
-  V8_EXPORT_PRIVATE void RemoveAllocationObserver(
-      AllocationObserver* observer) override;
-  V8_EXPORT_PRIVATE void ResumeAllocationObservers() override;
-
- protected:
-  // TODO(ofrobots): make these private after refactoring is complete.
-  AllocationInfo allocation_info_;
-  Address top_on_previous_step_;
-};
-
-class V8_EXPORT_PRIVATE PagedSpace
-    : NON_EXPORTED_BASE(public SpaceWithLinearArea) {
+class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
  public:
   typedef PageIterator iterator;
 
@@ -2081,6 +2038,18 @@ class V8_EXPORT_PRIVATE PagedSpace
   // due to being too small to use for allocation.
   virtual size_t Waste() { return free_list_.wasted_bytes(); }
 
+  // Returns the allocation pointer in this space.
+  Address top() { return allocation_info_.top(); }
+  Address limit() { return allocation_info_.limit(); }
+
+  // The allocation top address.
+  Address* allocation_top_address() { return allocation_info_.top_address(); }
+
+  // The allocation limit address.
+  Address* allocation_limit_address() {
+    return allocation_info_.limit_address();
+  }
+
   enum UpdateSkipList { UPDATE_SKIP_LIST, IGNORE_SKIP_LIST };
 
   // Allocate the requested number of bytes in the space if possible, return a
@@ -2121,7 +2090,13 @@ class V8_EXPORT_PRIVATE PagedSpace
 
   void ResetFreeList() { free_list_.Reset(); }
 
+  void AddAllocationObserver(AllocationObserver* observer) override;
+  void RemoveAllocationObserver(AllocationObserver* observer) override;
   void PauseAllocationObservers() override;
+  void ResumeAllocationObservers() override;
+
+  void InlineAllocationStep(Address top, Address new_top, Address soon_object,
+                            size_t size);
 
   // Empty space allocation info, returning unused area to free list.
   void EmptyAllocationInfo();
@@ -2296,9 +2271,14 @@ class V8_EXPORT_PRIVATE PagedSpace
   // The space's free list.
   FreeList free_list_;
 
+  // Normal allocation information.
+  AllocationInfo allocation_info_;
+
   // Mutex guarding any concurrent access to the space.
   base::Mutex space_mutex_;
 
+  Address top_on_previous_step_;
+
   friend class IncrementalMarking;
   friend class MarkCompactCollector;
 
@@ -2509,12 +2489,13 @@ class SemiSpaceIterator : public ObjectIterator {
 // The new space consists of a contiguous pair of semispaces.  It simply
 // forwards most functions to the appropriate semispace.
 
-class NewSpace : public SpaceWithLinearArea {
+class NewSpace : public Space {
  public:
   typedef PageIterator iterator;
 
   explicit NewSpace(Heap* heap)
-      : SpaceWithLinearArea(heap, NEW_SPACE, NOT_EXECUTABLE),
+      : Space(heap, NEW_SPACE, NOT_EXECUTABLE),
+        top_on_previous_step_(0),
         to_space_(heap, kToSpace),
         from_space_(heap, kFromSpace),
         reservation_(),
@@ -2639,6 +2620,18 @@ class NewSpace : public SpaceWithLinearArea {
     return to_space_.minimum_capacity();
   }
 
+  // Return the address of the allocation pointer in the active semispace.
+  Address top() {
+    DCHECK(to_space_.current_page()->ContainsLimit(allocation_info_.top()));
+    return allocation_info_.top();
+  }
+
+  // Return the address of the allocation pointer limit in the active semispace.
+  Address limit() {
+    DCHECK(to_space_.current_page()->ContainsLimit(allocation_info_.limit()));
+    return allocation_info_.limit();
+  }
+
   void ResetOriginalTop() {
     DCHECK_GE(top(), original_top());
     DCHECK_LE(top(), original_limit());
@@ -2656,6 +2649,14 @@ class NewSpace : public SpaceWithLinearArea {
   // Set the age mark in the active semispace.
   void set_age_mark(Address mark) { to_space_.set_age_mark(mark); }
 
+  // The allocation top and limit address.
+  Address* allocation_top_address() { return allocation_info_.top_address(); }
+
+  // The allocation limit address.
+  Address* allocation_limit_address() {
+    return allocation_info_.limit_address();
+  }
+
   MUST_USE_RESULT INLINE(AllocationResult AllocateRawAligned(
       int size_in_bytes, AllocationAlignment alignment));
 
@@ -2745,7 +2746,10 @@ class NewSpace : public SpaceWithLinearArea {
 
   SemiSpace* active_space() { return &to_space_; }
 
+  void AddAllocationObserver(AllocationObserver* observer) override;
+  void RemoveAllocationObserver(AllocationObserver* observer) override;
   void PauseAllocationObservers() override;
+  void ResumeAllocationObservers() override;
 
   iterator begin() { return to_space_.begin(); }
   iterator end() { return to_space_.end(); }
@@ -2761,6 +2765,10 @@ class NewSpace : public SpaceWithLinearArea {
 
   base::Mutex mutex_;
 
+  // Allocation pointer and limit for normal allocation and allocation during
+  // mark-compact collection.
+  AllocationInfo allocation_info_;
+  Address top_on_previous_step_;
   // The top and the limit at the time of setting the allocation info.
   // These values can be accessed by background tasks.
   base::AtomicValue<Address> original_top_;
@@ -2776,6 +2784,14 @@ class NewSpace : public SpaceWithLinearArea {
 
   bool EnsureAllocation(int size_in_bytes, AllocationAlignment alignment);
 
+  // If we are doing inline allocation in steps, this method performs the 'step'
+  // operation. top is the memory address of the bump pointer at the last
+  // inline allocation (i.e. it determines the numbers of bytes actually
+  // allocated since the last step.) new_top is the address of the bump pointer
+  // where the next byte is going to be allocated from. top and new_top may be
+  // different when we cross a page boundary or reset the space.
+  void InlineAllocationStep(Address top, Address new_top, Address soon_object,
+                            size_t size);
   void StartNextInlineAllocationStep() override;
 
   friend class SemiSpaceIterator;