Revert of Concurrent sweeping of code space. (patchset #4 id:60001 of https://coderev...

Reason for revert:
[Sheriff] Increased flaky crashes. See:
https://code.google.com/p/v8/issues/detail?id=4275

Original issue's description:
> Concurrent sweeping of code space.
>
> BUG=
>
> Committed: https://crrev.com/3050b52f57d652dc45c8baf416e174f22dc2c159
> Cr-Commit-Position: refs/heads/master@{#29456}

TBR=jochen@chromium.org,hpayer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/1223763003

Cr-Commit-Position: refs/heads/master@{#29475}

diff --git a/src/frames.cc b/src/frames.cc
index 7f4d7deaef18801675bd784aff036ad96ce33100..cde6f21f74813ddb7f90612293bcc057225929ae 100644 (file)
--- a/src/frames.cc
+++ b/src/frames.cc
@@ -1431,9 +1431,7 @@ Code* InnerPointerToCodeCache::GcSafeFindCodeForInnerPointer(
   // after the inner pointer.
   Page* page = Page::FromAddress(inner_pointer);
 
-  page->skip_list()->Lock();
   Address addr = page->skip_list()->StartFor(inner_pointer);
-  page->skip_list()->Unlock();
 
   Address top = heap->code_space()->top();
   Address limit = heap->code_space()->limit();

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index dc5c70aebf62c7b79c8e22e5bfe049593b9b2774..56a7941d261b0b0debb42b37f60c42639ca1f167 100644 (file)
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -5125,11 +5125,6 @@ void Heap::Verify() {
   code_space_->Verify(&no_dirty_regions_visitor);
 
   lo_space_->Verify();
-
-  mark_compact_collector_.VerifyWeakEmbeddedObjectsInCode();
-  if (FLAG_omit_map_checks_for_leaf_maps) {
-    mark_compact_collector_.VerifyOmittedMapChecks();
-  }
 }
 #endif
 

diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index 74e1b3f80f2e4331caa2bc0fa0326b7b2bde3434..8bbfeb173f7ba3ac5e55b63145e62d3085338727 100644 (file)
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -226,7 +226,6 @@ static void VerifyEvacuation(Heap* heap) {
 
 void MarkCompactCollector::SetUp() {
   free_list_old_space_.Reset(new FreeList(heap_->old_space()));
-  free_list_code_space_.Reset(new FreeList(heap_->code_space()));
   EnsureMarkingDequeIsReserved();
   EnsureMarkingDequeIsCommitted(kMinMarkingDequeSize);
 }
@@ -367,6 +366,13 @@ void MarkCompactCollector::CollectGarbage() {
 
   SweepSpaces();
 
+#ifdef VERIFY_HEAP
+  VerifyWeakEmbeddedObjectsInCode();
+  if (FLAG_omit_map_checks_for_leaf_maps) {
+    VerifyOmittedMapChecks();
+  }
+#endif
+
   Finish();
 
   if (marking_parity_ == EVEN_MARKING_PARITY) {
@@ -493,13 +499,9 @@ class MarkCompactCollector::SweeperTask : public v8::Task {
 
 void MarkCompactCollector::StartSweeperThreads() {
   DCHECK(free_list_old_space_.get()->IsEmpty());
-  DCHECK(free_list_code_space_.get()->IsEmpty());
   V8::GetCurrentPlatform()->CallOnBackgroundThread(
       new SweeperTask(heap(), heap()->old_space()),
       v8::Platform::kShortRunningTask);
-  V8::GetCurrentPlatform()->CallOnBackgroundThread(
-      new SweeperTask(heap(), heap()->code_space()),
-      v8::Platform::kShortRunningTask);
 }
 
 
@@ -510,19 +512,15 @@ void MarkCompactCollector::EnsureSweepingCompleted() {
   // here.
   if (!heap()->concurrent_sweeping_enabled() || !IsSweepingCompleted()) {
     SweepInParallel(heap()->paged_space(OLD_SPACE), 0);
-    SweepInParallel(heap()->paged_space(CODE_SPACE), 0);
   }
   // Wait twice for both jobs.
   if (heap()->concurrent_sweeping_enabled()) {
     pending_sweeper_jobs_semaphore_.Wait();
-    pending_sweeper_jobs_semaphore_.Wait();
   }
   ParallelSweepSpacesComplete();
   sweeping_in_progress_ = false;
   RefillFreeList(heap()->paged_space(OLD_SPACE));
-  RefillFreeList(heap()->paged_space(CODE_SPACE));
   heap()->paged_space(OLD_SPACE)->ResetUnsweptFreeBytes();
-  heap()->paged_space(CODE_SPACE)->ResetUnsweptFreeBytes();
 
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap && !evacuation()) {
@@ -547,8 +545,6 @@ void MarkCompactCollector::RefillFreeList(PagedSpace* space) {
 
   if (space == heap()->old_space()) {
     free_list = free_list_old_space_.get();
-  } else if (space == heap()->code_space()) {
-    free_list = free_list_code_space_.get();
   } else {
     // Any PagedSpace might invoke RefillFreeLists, so we need to make sure
     // to only refill them for the old space.
@@ -3503,20 +3499,14 @@ static int Sweep(PagedSpace* space, FreeList* free_list, Page* p,
   DCHECK(reinterpret_cast<intptr_t>(free_start) % (32 * kPointerSize) == 0);
   int offsets[16];
 
-  // If we use the skip list for code space pages, we have to lock the skip
-  // list because it could be accessed concurrently by the runtime or the
-  // deoptimizer.
-  bool skip_list_locked = false;
   SkipList* skip_list = p->skip_list();
+  int curr_region = -1;
   if ((skip_list_mode == REBUILD_SKIP_LIST) && skip_list) {
-    skip_list->Lock();
-    skip_list_locked = true;
     skip_list->Clear();
   }
 
   intptr_t freed_bytes = 0;
   intptr_t max_freed_bytes = 0;
-  int curr_region = -1;
 
   for (MarkBitCellIterator it(p); !it.Done(); it.Advance()) {
     Address cell_base = it.CurrentCellBase();
@@ -3571,10 +3561,6 @@ static int Sweep(PagedSpace* space, FreeList* free_list, Page* p,
   } else {
     p->SetWasSwept();
   }
-  if (skip_list_locked) {
-    DCHECK(skip_list && skip_list_mode == REBUILD_SKIP_LIST);
-    skip_list->Unlock();
-  }
   return FreeList::GuaranteedAllocatable(static_cast<int>(max_freed_bytes));
 }
 
@@ -4184,19 +4170,10 @@ int MarkCompactCollector::SweepInParallel(PagedSpace* space,
 int MarkCompactCollector::SweepInParallel(Page* page, PagedSpace* space) {
   int max_freed = 0;
   if (page->TryParallelSweeping()) {
-    FreeList* free_list;
+    FreeList* free_list = free_list_old_space_.get();
     FreeList private_free_list(space);
-    if (space->identity() == CODE_SPACE) {
-      free_list = free_list_code_space_.get();
-      max_freed =
-          Sweep<SWEEP_ONLY, SWEEP_IN_PARALLEL, REBUILD_SKIP_LIST,
-                IGNORE_FREE_SPACE>(space, &private_free_list, page, NULL);
-    } else {
-      free_list = free_list_old_space_.get();
-      max_freed =
-          Sweep<SWEEP_ONLY, SWEEP_IN_PARALLEL, IGNORE_SKIP_LIST,
-                IGNORE_FREE_SPACE>(space, &private_free_list, page, NULL);
-    }
+    max_freed = Sweep<SWEEP_ONLY, SWEEP_IN_PARALLEL, IGNORE_SKIP_LIST,
+                      IGNORE_FREE_SPACE>(space, &private_free_list, page, NULL);
     free_list->Concatenate(&private_free_list);
   }
   return max_freed;
@@ -4253,19 +4230,8 @@ void MarkCompactCollector::SweepSpace(PagedSpace* space, SweeperType sweeper) {
             PrintF("Sweeping 0x%" V8PRIxPTR ".\n",
                    reinterpret_cast<intptr_t>(p));
           }
-          if (space->identity() == CODE_SPACE) {
-            if (FLAG_zap_code_space) {
-              Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
-                    ZAP_FREE_SPACE>(space, NULL, p, NULL);
-            } else {
-              Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
-                    IGNORE_FREE_SPACE>(space, NULL, p, NULL);
-            }
-          } else {
-            DCHECK(space->identity() == OLD_SPACE);
-            Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, IGNORE_SKIP_LIST,
-                  IGNORE_FREE_SPACE>(space, NULL, p, NULL);
-          }
+          Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, IGNORE_SKIP_LIST,
+                IGNORE_FREE_SPACE>(space, NULL, p, NULL);
           pages_swept++;
           parallel_sweeping_active = true;
         } else {
@@ -4282,17 +4248,13 @@ void MarkCompactCollector::SweepSpace(PagedSpace* space, SweeperType sweeper) {
         if (FLAG_gc_verbose) {
           PrintF("Sweeping 0x%" V8PRIxPTR ".\n", reinterpret_cast<intptr_t>(p));
         }
-        if (space->identity() == CODE_SPACE) {
-          if (FLAG_zap_code_space) {
-            Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
-                  ZAP_FREE_SPACE>(space, NULL, p, NULL);
-          } else {
-            Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
-                  IGNORE_FREE_SPACE>(space, NULL, p, NULL);
-          }
+        if (space->identity() == CODE_SPACE && FLAG_zap_code_space) {
+          Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
+                ZAP_FREE_SPACE>(space, NULL, p, NULL);
+        } else if (space->identity() == CODE_SPACE) {
+          Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
+                IGNORE_FREE_SPACE>(space, NULL, p, NULL);
         } else {
-          DCHECK(space->identity() == OLD_SPACE ||
-                 space->identity() == MAP_SPACE);
           Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, IGNORE_SKIP_LIST,
                 IGNORE_FREE_SPACE>(space, NULL, p, NULL);
         }
@@ -4332,22 +4294,19 @@ void MarkCompactCollector::SweepSpaces() {
   // the other spaces rely on possibly non-live maps to get the sizes for
   // non-live objects.
   {
-    {
-      GCTracer::Scope sweep_scope(heap()->tracer(),
-                                  GCTracer::Scope::MC_SWEEP_OLDSPACE);
-      SweepSpace(heap()->old_space(), CONCURRENT_SWEEPING);
-    }
-    {
-      GCTracer::Scope sweep_scope(heap()->tracer(),
-                                  GCTracer::Scope::MC_SWEEP_CODE);
-      SweepSpace(heap()->code_space(), CONCURRENT_SWEEPING);
-    }
-
+    GCTracer::Scope sweep_scope(heap()->tracer(),
+                                GCTracer::Scope::MC_SWEEP_OLDSPACE);
+    { SweepSpace(heap()->old_space(), CONCURRENT_SWEEPING); }
     sweeping_in_progress_ = true;
     if (heap()->concurrent_sweeping_enabled()) {
       StartSweeperThreads();
     }
   }
+  {
+    GCTracer::Scope sweep_scope(heap()->tracer(),
+                                GCTracer::Scope::MC_SWEEP_CODE);
+    SweepSpace(heap()->code_space(), SEQUENTIAL_SWEEPING);
+  }
 
   EvacuateNewSpaceAndCandidates();
 
@@ -4400,7 +4359,6 @@ void MarkCompactCollector::ParallelSweepSpaceComplete(PagedSpace* space) {
 
 void MarkCompactCollector::ParallelSweepSpacesComplete() {
   ParallelSweepSpaceComplete(heap()->old_space());
-  ParallelSweepSpaceComplete(heap()->code_space());
 }
 
 

diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
index f7dfedd93f876e8a0e5620bb3da4d13895479bde..9892e0e42c89c4d67d675b4b465d4893df05a33e 100644 (file)
--- a/src/heap/mark-compact.h
+++ b/src/heap/mark-compact.h
@@ -970,7 +970,6 @@ class MarkCompactCollector {
   List<Page*> evacuation_candidates_;
 
   SmartPointer<FreeList> free_list_old_space_;
-  SmartPointer<FreeList> free_list_code_space_;
 
   friend class Heap;
 };

diff --git a/src/heap/spaces.h b/src/heap/spaces.h
index 4a460b0636bf70d92d65aaae0cf6145cb2bb2e70..b880da95c2ca08305b4f316a0102f97068f491ce 100644 (file)
--- a/src/heap/spaces.h
+++ b/src/heap/spaces.h
@@ -1014,10 +1014,6 @@ class SkipList {
     list->AddObject(addr, size);
   }
 
-  void Lock() { mutex_.Lock(); }
-
-  void Unlock() { mutex_.Unlock(); }
-
  private:
   static const int kRegionSizeLog2 = 13;
   static const int kRegionSize = 1 << kRegionSizeLog2;
@@ -1026,7 +1022,6 @@ class SkipList {
   STATIC_ASSERT(Page::kPageSize % kRegionSize == 0);
 
   Address starts_[kSize];
-  base::Mutex mutex_;
 };