[heap] Re-orgainize roots marking

Move roots marking into ThreadState as most necessary data structures
do not live in ThreadHeap but ThreadState.

Bug: chromium:843903
Change-Id: I5e4f56f594911ebc0d9a17dcdfb4834c6befa555
Reviewed-on: https://chromium-review.googlesource.com/1189864
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586272}

third_party/blink/renderer/platform/heap/address_cache.cc

@@ -8,6 +8,16 @@
 
 namespace blink {
 
+AddressCache::EnabledScope::EnabledScope(AddressCache* address_cache)
+    : address_cache_(address_cache) {
+  address_cache_->FlushIfDirty();
+  address_cache_->EnableLookup();
+}
+
+AddressCache::EnabledScope::~EnabledScope() {
+  address_cache_->DisableLookup();
+}
+
 void AddressCache::Flush() {
   if (has_entries_) {
     for (size_t i = 0; i < kNumberOfEntries; ++i)

third_party/blink/renderer/platform/heap/address_cache.h

@@ -16,12 +16,17 @@ class PLATFORM_EXPORT AddressCache {
   USING_FAST_MALLOC(AddressCache);
 
  public:
-  AddressCache() : enabled_(false), has_entries_(false), dirty_(false) {
-    // Start by flushing the cache in a non-empty state to initialize all the
-    // cache entries.
-    for (size_t i = 0; i < kNumberOfEntries; ++i)
-      entries_[i] = nullptr;
-  }
+  class PLATFORM_EXPORT EnabledScope {
+   public:
+    explicit EnabledScope(AddressCache*);
+    ~EnabledScope();
+
+   private:
+    AddressCache* const address_cache_;
+  };
+
+  AddressCache()
+      : entries_{}, enabled_(false), has_entries_(false), dirty_(false) {}
 
   void EnableLookup() { enabled_ = true; }
   void DisableLookup() { enabled_ = false; }
@@ -29,7 +34,7 @@ class PLATFORM_EXPORT AddressCache {
   void MarkDirty() { dirty_ = true; }
   void Flush();
   void FlushIfDirty();
-  bool IsEmpty() { return !has_entries_; }
+  bool IsEmpty() const { return !has_entries_; }
 
   // Perform a lookup in the cache. Returns true if the address is guaranteed
   // to not in Blink's heap and false otherwise.

third_party/blink/renderer/platform/heap/address_cache_test.cc

@@ -15,6 +15,12 @@ const Address kObjectAddress = reinterpret_cast<Address>(kBlinkPageSize);
 
 }  // namespace
 
+TEST(AddressCacheTest, Scope) {
+  AddressCache cache;
+  AddressCache::EnabledScope scope(&cache);
+  EXPECT_FALSE(cache.Lookup(kObjectAddress));
+}
+
 TEST(AddressCacheTest, InitialIsEmpty) {
   AddressCache cache;
   cache.EnableLookup();

third_party/blink/renderer/platform/heap/heap.cc

@@ -312,23 +312,6 @@ size_t ThreadHeap::ObjectPayloadSizeForTesting() {
   return object_payload_size;
 }
 
-void ThreadHeap::VisitPersistentRoots(Visitor* visitor) {
-  ThreadHeapStatsCollector::Scope stats_scope(
-      stats_collector(), ThreadHeapStatsCollector::kVisitPersistentRoots);
-  DCHECK(thread_state_->InAtomicMarkingPause());
-  thread_state_->VisitPersistents(visitor);
-}
-
-void ThreadHeap::VisitStackRoots() {
-  ThreadHeapStatsCollector::Scope stats_scope(
-      stats_collector(), ThreadHeapStatsCollector::kVisitStackRoots);
-  DCHECK(thread_state_->InAtomicMarkingPause());
-  address_cache_->FlushIfDirty();
-  address_cache_->EnableLookup();
-  thread_state_->PushRegistersAndVisitStack();
-  address_cache_->DisableLookup();
-}
-
 BasePage* ThreadHeap::LookupPageForAddress(Address address) {
   if (PageMemoryRegion* region = region_tree_->Lookup(address)) {
     return region->PageFromAddress(address);

third_party/blink/renderer/platform/heap/heap.h

@@ -203,9 +203,6 @@ class PLATFORM_EXPORT ThreadHeap {
     return weak_callback_worklist_.get();
   }
 
-  void VisitPersistentRoots(Visitor*);
-  void VisitStackRoots();
-
   // Is the finalizable GC object still alive, but slated for lazy sweeping?
   // If a lazy sweep is in progress, returns true if the object was found
   // to be not reachable during the marking phase, but it has yet to be swept
@@ -304,7 +301,7 @@ class PLATFORM_EXPORT ThreadHeap {
 
   size_t ObjectPayloadSizeForTesting();
 
-  AddressCache* address_cache() { return address_cache_.get(); }
+  AddressCache* address_cache() const { return address_cache_.get(); }
 
   PagePool* GetFreePagePool() { return free_page_pool_.get(); }
 

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -43,6 +43,7 @@
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/web_thread.h"
 #include "third_party/blink/renderer/platform/bindings/runtime_call_stats.h"
+#include "third_party/blink/renderer/platform/heap/address_cache.h"
 #include "third_party/blink/renderer/platform/heap/blink_gc_memory_dump_provider.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
@@ -366,7 +367,18 @@ void ThreadState::VisitStack(MarkingVisitor* visitor) {
 #endif
 }
 
+void ThreadState::VisitDOMWrappers(Visitor* visitor) {
+  if (trace_dom_wrappers_) {
+    ThreadHeapStatsCollector::Scope stats_scope(
+        Heap().stats_collector(), ThreadHeapStatsCollector::kVisitDOMWrappers);
+    trace_dom_wrappers_(isolate_, visitor);
+  }
+}
+
 void ThreadState::VisitPersistents(Visitor* visitor) {
+  ThreadHeapStatsCollector::Scope stats_scope(
+      Heap().stats_collector(),
+      ThreadHeapStatsCollector::kVisitPersistentRoots);
   {
     ThreadHeapStatsCollector::Scope stats_scope(
         Heap().stats_collector(),
@@ -380,11 +392,6 @@ void ThreadState::VisitPersistents(Visitor* visitor) {
         Heap().stats_collector(), ThreadHeapStatsCollector::kVisitPersistents);
     persistent_region_->TracePersistentNodes(visitor);
   }
-  if (trace_dom_wrappers_) {
-    ThreadHeapStatsCollector::Scope stats_scope(
-        Heap().stats_collector(), ThreadHeapStatsCollector::kVisitDOMWrappers);
-    trace_dom_wrappers_(isolate_, visitor);
-  }
 }
 
 void ThreadState::VisitWeakPersistents(Visitor* visitor) {
@@ -1606,16 +1613,22 @@ void ThreadState::AtomicPausePrologue(BlinkGC::StackState stack_state,
 }
 
 void ThreadState::MarkPhaseVisitRoots() {
-  // StackFrameDepth should be disabled so we don't trace most of the object
-  // graph in one incremental marking step.
+  // StackFrameDepth should be disabled to avoid eagerly tracing into the object
+  // graph when just visiting roots.
   DCHECK(!Heap().GetStackFrameDepth().IsEnabled());
 
-  // 1. Trace persistent roots.
-  Heap().VisitPersistentRoots(current_gc_data_.visitor.get());
+  Visitor* visitor = current_gc_data_.visitor.get();
 
-  // 2. Trace objects reachable from the stack.
-  if (current_gc_data_.stack_state == BlinkGC::kHeapPointersOnStack)
-    Heap().VisitStackRoots();
+  VisitPersistents(visitor);
+
+  VisitDOMWrappers(visitor);
+
+  if (current_gc_data_.stack_state == BlinkGC::kHeapPointersOnStack) {
+    ThreadHeapStatsCollector::Scope stats_scope(
+        Heap().stats_collector(), ThreadHeapStatsCollector::kVisitStackRoots);
+    AddressCache::EnabledScope address_cache_scope(Heap().address_cache());
+    PushRegistersAndVisitStack();
+  }
 }
 
 bool ThreadState::MarkPhaseAdvanceMarking(TimeTicks deadline) {

third_party/blink/renderer/platform/heap/thread_state.h

@@ -443,6 +443,9 @@ class PLATFORM_EXPORT ThreadState final
   // Visit all weak persistents allocated on this thread.
   void VisitWeakPersistents(Visitor*);
 
+  // Visit all DOM wrappers allocatd on this thread.
+  void VisitDOMWrappers(Visitor*);
+
   struct GCSnapshotInfo {
     STACK_ALLOCATED();