[heap] Remove anchor page from Space.

Replaces the anchor page circular doubly linked list
with a doubly linked list pointing to nullptr on its ends.

Fixes a memory leak when rewinding pages.

The large pages list will move to the new list implementation
in a follow-up CL.

Change-Id: I2933a5e222d4ca768f4b555c47ed0d7a7027aa73
Reviewed-on: https://chromium-review.googlesource.com/1060973
Commit-Queue: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53288}

BUILD.gn

@@ -2814,6 +2814,7 @@ v8_component("v8_libbase") {
     "src/base/ieee754.h",
     "src/base/iterator.h",
     "src/base/lazy-instance.h",
+    "src/base/list.h",
     "src/base/logging.cc",
     "src/base/logging.h",
     "src/base/macros.h",

src/base/list.h

+// Copyright 2018 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.
+
+#ifndef V8_BASE_LIST_H_
+#define V8_BASE_LIST_H_
+
+#include <atomic>
+
+#include "src/base/logging.h"
+
+// TODO(hpayer): Remove as soon LargePage was ported to use List.
+namespace v8 {
+namespace internal {
+class LargePage;
+}
+}  // namespace v8
+
+namespace v8 {
+namespace base {
+
+template <class T>
+class List {
+ public:
+  List() : front_(nullptr), back_(nullptr) {}
+
+  void PushBack(T* element) {
+    DCHECK(!element->list_node().next());
+    DCHECK(!element->list_node().prev());
+    if (back_) {
+      DCHECK(front_);
+      InsertAfter(element, back_);
+    } else {
+      AddFirstElement(element);
+    }
+  }
+
+  void PushFront(T* element) {
+    DCHECK(!element->list_node().next());
+    DCHECK(!element->list_node().prev());
+    if (front_) {
+      DCHECK(back_);
+      InsertBefore(element, front_);
+    } else {
+      AddFirstElement(element);
+    }
+  }
+
+  void Remove(T* element) {
+    DCHECK(Contains(element));
+    if (back_ == element) {
+      back_ = element->list_node().prev();
+    }
+    if (front_ == element) {
+      front_ = element->list_node().next();
+    }
+    T* next = element->list_node().next();
+    T* prev = element->list_node().prev();
+    if (next) next->list_node().set_prev(prev);
+    if (prev) prev->list_node().set_next(next);
+    element->list_node().set_prev(nullptr);
+    element->list_node().set_next(nullptr);
+  }
+
+  bool Contains(T* element) {
+    T* it = front_;
+    while (it) {
+      if (it == element) return true;
+      it = it->list_node().next();
+    }
+    return false;
+  }
+
+  bool Empty() { return !front_ && !back_; }
+
+  T* front() { return front_; }
+  T* back() { return back_; }
+
+ private:
+  void AddFirstElement(T* element) {
+    DCHECK(!back_);
+    DCHECK(!front_);
+    DCHECK(!element->list_node().next());
+    DCHECK(!element->list_node().prev());
+    element->list_node().set_prev(nullptr);
+    element->list_node().set_next(nullptr);
+    front_ = element;
+    back_ = element;
+  }
+
+  void InsertAfter(T* element, T* other) {
+    T* other_next = other->list_node().next();
+    element->list_node().set_next(other_next);
+    element->list_node().set_prev(other);
+    other->list_node().set_next(element);
+    if (other_next)
+      other_next->list_node().set_prev(element);
+    else
+      back_ = element;
+  }
+
+  void InsertBefore(T* element, T* other) {
+    T* other_prev = other->list_node().prev();
+    element->list_node().set_next(other);
+    element->list_node().set_prev(other_prev);
+    other->list_node().set_prev(element);
+    if (other_prev) {
+      other_prev->list_node().set_next(element);
+    } else {
+      front_ = element;
+    }
+  }
+
+  T* front_;
+  T* back_;
+};
+
+template <class T>
+class ListNode {
+ public:
+  ListNode() { Initialize(); }
+
+  T* next() { return next_; }
+  T* prev() { return prev_; }
+
+  void Initialize() {
+    next_ = nullptr;
+    prev_ = nullptr;
+  }
+
+ private:
+  void set_next(T* next) { next_ = next; }
+  void set_prev(T* prev) { prev_ = prev; }
+
+  T* next_;
+  T* prev_;
+
+  friend class List<T>;
+  // TODO(hpayer): Remove as soon LargePage was ported to use List.
+  friend class v8::internal::LargePage;
+};
+}  // namespace base
+}  // namespace v8
+
+#endif  // V8_BASE_LIST_H_

src/heap/heap-inl.h

@@ -489,13 +489,12 @@ AllocationMemento* Heap::FindAllocationMemento(Map* map, HeapObject* object) {
 void Heap::UpdateAllocationSite(Map* map, HeapObject* object,
                                 PretenuringFeedbackMap* pretenuring_feedback) {
   DCHECK_NE(pretenuring_feedback, &global_pretenuring_feedback_);
-  DCHECK(InFromSpace(object) ||
-         (InToSpace(object) &&
-          Page::FromAddress(object->address())
-              ->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION)) ||
-         (!InNewSpace(object) &&
-          Page::FromAddress(object->address())
-              ->IsFlagSet(Page::PAGE_NEW_OLD_PROMOTION)));
+  DCHECK(
+      InFromSpace(object) ||
+      (InToSpace(object) && Page::FromAddress(object->address())
+                                ->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION)) ||
+      (!InNewSpace(object) && Page::FromAddress(object->address())
+                                  ->IsFlagSet(Page::PAGE_NEW_OLD_PROMOTION)));
   if (!FLAG_allocation_site_pretenuring ||
       !AllocationSite::CanTrack(map->instance_type()))
     return;

src/heap/heap.cc

@@ -1978,7 +1978,7 @@ void Heap::EvacuateYoungGeneration() {
   PageRange range(new_space()->bottom(), new_space()->top());
   for (auto it = range.begin(); it != range.end();) {
     Page* p = (*++it)->prev_page();
-    p->Unlink();
+    new_space()->from_space().RemovePage(p);
     Page::ConvertNewToOld(p);
     if (incremental_marking()->IsMarking())
       mark_compact_collector()->RecordLiveSlotsOnPage(p);
@@ -4530,7 +4530,7 @@ HeapObject* Heap::EnsureImmovableCode(HeapObject* heap_object,
   DCHECK_GE(object_size, 0);
   if (!Heap::IsImmovable(heap_object)) {
     if (isolate()->serializer_enabled() ||
-        code_space_->FirstPage()->Contains(heap_object->address())) {
+        code_space_->first_page()->Contains(heap_object->address())) {
       MemoryChunk::FromAddress(heap_object->address())->MarkNeverEvacuate();
     } else {
       // Discard the first code allocation, which was on a page where it could

src/heap/mark-compact.cc

@@ -1365,7 +1365,7 @@ class EvacuateNewSpacePageVisitor final : public HeapObjectVisitor {
         page->SetFlag(Page::PAGE_NEW_NEW_PROMOTION);
         break;
       case NEW_TO_OLD: {
-        page->Unlink();
+        page->heap()->new_space()->from_space().RemovePage(page);
         Page* new_page = Page::ConvertNewToOld(page);
         DCHECK(!new_page->InNewSpace());
         new_page->SetFlag(Page::PAGE_NEW_OLD_PROMOTION);
@@ -3160,7 +3160,7 @@ void MarkCompactCollector::PostProcessEvacuationCandidates() {
     } else {
       DCHECK(p->IsEvacuationCandidate());
       DCHECK(p->SweepingDone());
-      p->Unlink();
+      p->owner()->memory_chunk_list().Remove(p);
     }
   }
   DCHECK_EQ(aborted_pages_verified, aborted_pages);
@@ -3221,6 +3221,7 @@ void MarkCompactCollector::StartSweepSpace(PagedSpace* space) {
                        static_cast<void*>(p));
         }
         ArrayBufferTracker::FreeAll(p);
+        space->memory_chunk_list().Remove(p);
         space->ReleasePage(p);
         continue;
       }

src/heap/spaces-inl.h

@@ -45,7 +45,7 @@ HeapObject* SemiSpaceIterator::Next() {
     if (Page::IsAlignedToPageSize(current_)) {
       Page* page = Page::FromAllocationAreaAddress(current_);
       page = page->next_page();
-      DCHECK(!page->is_anchor());
+      DCHECK(page);
       current_ = page->area_start();
       if (current_ == limit_) return nullptr;
     }

src/snapshot/deserializer.cc

@@ -286,7 +286,7 @@ HeapObject* Deserializer<AllocatorT>::GetBackReferencedObject(int space) {
     case RO_SPACE:
       if (isolate()->heap()->deserialization_complete()) {
         PagedSpace* read_only_space = isolate()->heap()->read_only_space();
-        Page* page = read_only_space->FirstPage();
+        Page* page = read_only_space->first_page();
         for (uint32_t i = 0; i < back_reference.chunk_index(); ++i) {
           page = page->next_page();
         }

test/cctest/heap/test-heap.cc

@@ -5817,7 +5817,7 @@ HEAP_TEST(Regress5831) {
   // Generate the code.
   Handle<Code> code = GenerateDummyImmovableCode(isolate);
   CHECK_GE(i::kMaxRegularHeapObjectSize, code->Size());
-  CHECK(!heap->code_space()->FirstPage()->Contains(code->address()));
+  CHECK(!heap->code_space()->first_page()->Contains(code->address()));
 
   // Ensure it's not in large object space.
   MemoryChunk* chunk = MemoryChunk::FromAddress(code->address());

test/cctest/heap/test-spaces.cc

@@ -209,15 +209,17 @@ TEST(MemoryAllocator) {
   {
     int total_pages = 0;
     OldSpace faked_space(heap);
+    CHECK(!faked_space.first_page());
+    CHECK(!faked_space.last_page());
     Page* first_page = memory_allocator->AllocatePage(
         faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space),
         NOT_EXECUTABLE);
 
-    first_page->InsertAfter(faked_space.anchor()->prev_page());
-    CHECK(first_page->next_page() == faked_space.anchor());
+    faked_space.memory_chunk_list().PushBack(first_page);
+    CHECK(first_page->next_page() == nullptr);
     total_pages++;
 
-    for (Page* p = first_page; p != faked_space.anchor(); p = p->next_page()) {
+    for (Page* p = first_page; p != nullptr; p = p->next_page()) {
       CHECK(p->owner() == &faked_space);
     }
 
@@ -226,9 +228,9 @@ TEST(MemoryAllocator) {
         faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space),
         NOT_EXECUTABLE);
     total_pages++;
-    other->InsertAfter(first_page);
+    faked_space.memory_chunk_list().PushBack(other);
     int page_count = 0;
-    for (Page* p = first_page; p != faked_space.anchor(); p = p->next_page()) {
+    for (Page* p = first_page; p != nullptr; p = p->next_page()) {
       CHECK(p->owner() == &faked_space);
       page_count++;
     }

test/cctest/test-weakmaps.cc

@@ -163,7 +163,7 @@ TEST(Regress2060a) {
   Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
 
   // Start second old-space page so that values land on evacuation candidate.
-  Page* first_page = heap->old_space()->anchor()->next_page();
+  Page* first_page = heap->old_space()->first_page();
   heap::SimulateFullSpace(heap->old_space());
 
   // Fill up weak map with values on an evacuation candidate.
@@ -202,7 +202,7 @@ TEST(Regress2060b) {
       factory->NewFunctionForTest(factory->function_string());
 
   // Start second old-space page so that keys land on evacuation candidate.
-  Page* first_page = heap->old_space()->anchor()->next_page();
+  Page* first_page = heap->old_space()->first_page();
   heap::SimulateFullSpace(heap->old_space());
 
   // Fill up weak map with keys on an evacuation candidate.

test/cctest/test-weaksets.cc

@@ -174,7 +174,7 @@ TEST(WeakSet_Regress2060a) {
   Handle<JSWeakSet> weakset = AllocateJSWeakSet(isolate);
 
   // Start second old-space page so that values land on evacuation candidate.
-  Page* first_page = heap->old_space()->anchor()->next_page();
+  Page* first_page = heap->old_space()->first_page();
   heap::SimulateFullSpace(heap->old_space());
 
   // Fill up weak set with values on an evacuation candidate.
@@ -213,7 +213,7 @@ TEST(WeakSet_Regress2060b) {
       factory->NewFunctionForTest(factory->function_string());
 
   // Start second old-space page so that keys land on evacuation candidate.
-  Page* first_page = heap->old_space()->anchor()->next_page();
+  Page* first_page = heap->old_space()->first_page();
   heap::SimulateFullSpace(heap->old_space());
 
   // Fill up weak set with keys on an evacuation candidate.

test/unittests/BUILD.gn

@@ -63,6 +63,7 @@ v8_source_set("unittests_sources") {
     "base/functional-unittest.cc",
     "base/ieee754-unittest.cc",
     "base/iterator-unittest.cc",
+    "base/list-unittest.cc",
     "base/logging-unittest.cc",
     "base/macros-unittest.cc",
     "base/ostreams-unittest.cc",

test/unittests/base/list-unittest.cc

+// Copyright 2018 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/base/list.h"
+#include "testing/gtest-support.h"
+
+namespace v8 {
+namespace base {
+
+class TestChunk {
+ public:
+  base::ListNode<TestChunk>& list_node() { return list_node_; }
+  base::ListNode<TestChunk> list_node_;
+};
+
+TEST(List, InsertAtTailAndRemove) {
+  List<TestChunk> list;
+  EXPECT_TRUE(list.Empty());
+  TestChunk t1;
+  list.PushBack(&t1);
+  EXPECT_FALSE(list.Empty());
+  EXPECT_TRUE(list.Contains(&t1));
+  list.Remove(&t1);
+  EXPECT_TRUE(list.Empty());
+}
+
+TEST(List, InsertAtHeadAndRemove) {
+  List<TestChunk> list;
+  EXPECT_TRUE(list.Empty());
+  TestChunk t1;
+  list.PushFront(&t1);
+  EXPECT_FALSE(list.Empty());
+  list.Remove(&t1);
+  EXPECT_TRUE(list.Empty());
+}
+
+TEST(List, InsertMultipleAtTailAndRemoveFromTail) {
+  List<TestChunk> list;
+  EXPECT_TRUE(list.Empty());
+  const int kSize = 10;
+  TestChunk chunks[kSize];
+  for (int i = 0; i < kSize; i++) {
+    list.PushBack(&chunks[i]);
+    EXPECT_EQ(list.back(), &chunks[i]);
+  }
+  for (int i = kSize - 1; i > 0; i--) {
+    list.Remove(&chunks[i]);
+    EXPECT_EQ(list.back(), &chunks[i - 1]);
+  }
+
+  list.Remove(&chunks[0]);
+  EXPECT_TRUE(list.Empty());
+}
+
+TEST(List, InsertMultipleAtHeadAndRemoveFromHead) {
+  List<TestChunk> list;
+  EXPECT_TRUE(list.Empty());
+  const int kSize = 10;
+  TestChunk chunks[kSize];
+  for (int i = 0; i < kSize; i++) {
+    list.PushFront(&chunks[i]);
+    EXPECT_EQ(list.front(), &chunks[i]);
+  }
+  for (int i = kSize - 1; i > 0; i--) {
+    list.Remove(&chunks[i]);
+    EXPECT_EQ(list.front(), &chunks[i - 1]);
+  }
+
+  list.Remove(&chunks[0]);
+  EXPECT_TRUE(list.Empty());
+}
+
+TEST(List, InsertMultipleAtTailAndRemoveFromMiddle) {
+  List<TestChunk> list;
+  EXPECT_TRUE(list.Empty());
+  const int kSize = 10;
+  TestChunk chunks[kSize];
+  for (int i = 0; i < kSize; i++) {
+    list.PushBack(&chunks[i]);
+    EXPECT_EQ(list.back(), &chunks[i]);
+  }
+  int i, j;
+  for (i = kSize / 2 - 1, j = kSize / 2; i >= 0; i--, j++) {
+    list.Remove(&chunks[i]);
+    list.Remove(&chunks[j]);
+  }
+  EXPECT_TRUE(list.Empty());
+}
+
+}  // namespace base
+}  // namespace v8