Revert of [heap] Uncommit marking deque in concurrent task. (patchset #7...

Revert of [heap] Uncommit marking deque in concurrent task. (patchset #7 id:120001 of https://codereview.chromium.org/2442443003/ )

Reason for revert:
Seems to break the world, e.g.:
https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/14118

Original issue's description:
> [heap] Uncommit marking deque in concurrent task.
>
> BUG=

TBR=mlippautz@chromium.org,ulan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review-Url: https://codereview.chromium.org/2454693002
Cr-Commit-Position: refs/heads/master@{#40588}

src/heap/mark-compact.cc

@@ -58,7 +58,6 @@ MarkCompactCollector::MarkCompactCollector(Heap* heap)
       compacting_(false),
       black_allocation_(false),
       have_code_to_deoptimize_(false),
-      marking_deque_(heap),
       code_flusher_(nullptr),
       sweeper_(heap) {
 }
@@ -2118,13 +2117,9 @@ void MarkingDeque::SetUp() {
   }
 }
 
-void MarkingDeque::TearDown() {
-  CancelOrWaitForUncommitTask();
-  delete backing_store_;
-}
+void MarkingDeque::TearDown() { delete backing_store_; }
 
 void MarkingDeque::StartUsing() {
-  base::LockGuard<base::Mutex> guard(&mutex_);
   if (in_use_) {
     // This can happen in mark-compact GC if the incremental marker already
     // started using the marking deque.
@@ -2144,16 +2139,11 @@ void MarkingDeque::StartUsing() {
 }
 
 void MarkingDeque::StopUsing() {
-  base::LockGuard<base::Mutex> guard(&mutex_);
   DCHECK(IsEmpty());
   DCHECK(!overflowed_);
   top_ = bottom_ = mask_ = 0;
+  Uncommit();
   in_use_ = false;
-  if (FLAG_concurrent_sweeping) {
-    StartUncommitTask();
-  } else {
-    Uncommit();
-  }
 }
 
 void MarkingDeque::Clear() {
@@ -2163,7 +2153,7 @@ void MarkingDeque::Clear() {
 }
 
 void MarkingDeque::Uncommit() {
-  DCHECK(!in_use_);
+  DCHECK(in_use_);
   bool success = backing_store_->Uncommit(backing_store_->address(),
                                           backing_store_committed_size_);
   backing_store_committed_size_ = 0;
@@ -2185,28 +2175,6 @@ void MarkingDeque::EnsureCommitted() {
   }
 }
 
-void MarkingDeque::StartUncommitTask() {
-  if (!uncommit_task_pending_) {
-    UncommitTask* task = new UncommitTask(heap_->isolate(), this);
-    uncommit_task_id_ = task->id();
-    uncommit_task_pending_ = true;
-    V8::GetCurrentPlatform()->CallOnBackgroundThread(
-        task, v8::Platform::kShortRunningTask);
-  }
-}
-
-void MarkingDeque::CancelOrWaitForUncommitTask() {
-  base::LockGuard<base::Mutex> guard(&mutex_);
-  if (!uncommit_task_pending_ ||
-      heap_->isolate()->cancelable_task_manager()->TryAbort(
-          uncommit_task_id_)) {
-    return;
-  }
-  while (uncommit_task_pending_) {
-    uncommit_task_barrier_.Wait(&mutex_);
-  }
-}
-
 class MarkCompactCollector::ObjectStatsVisitor
     : public MarkCompactCollector::HeapObjectVisitor {
  public:

src/heap/mark-compact.h

@@ -8,8 +8,6 @@
 #include <deque>
 
 #include "src/base/bits.h"
-#include "src/base/platform/condition-variable.h"
-#include "src/cancelable-task.h"
 #include "src/heap/marking.h"
 #include "src/heap/spaces.h"
 #include "src/heap/store-buffer.h"
@@ -54,7 +52,7 @@ class ObjectMarking : public AllStatic {
 // Marking deque for tracing live objects.
 class MarkingDeque {
  public:
-  explicit MarkingDeque(Heap* heap)
+  MarkingDeque()
       : backing_store_(nullptr),
         backing_store_committed_size_(0),
         array_(nullptr),
@@ -62,16 +60,11 @@ class MarkingDeque {
         bottom_(0),
         mask_(0),
         overflowed_(false),
-        in_use_(false),
-        uncommit_task_pending_(false),
-        uncommit_task_id_(0),
-        heap_(heap) {}
+        in_use_(false) {}
 
   void SetUp();
   void TearDown();
 
-  // Ensures that the marking deque is committed and will stay committed until
-  // StopUsing() is called.
   void StartUsing();
   void StopUsing();
   void Clear();
@@ -129,45 +122,12 @@ class MarkingDeque {
   void set_top(int top) { top_ = top; }
 
  private:
-  // This task uncommits the marking_deque backing store if
-  // markin_deque->in_use_ is false.
-  class UncommitTask : public CancelableTask {
-   public:
-    explicit UncommitTask(Isolate* isolate, MarkingDeque* marking_deque)
-        : CancelableTask(isolate), marking_deque_(marking_deque) {}
-
-   private:
-    // CancelableTask override.
-    void RunInternal() override {
-      base::LockGuard<base::Mutex> guard(&marking_deque_->mutex_);
-      if (!marking_deque_->in_use_) {
-        marking_deque_->Uncommit();
-      }
-      marking_deque_->uncommit_task_pending_ = false;
-      marking_deque_->uncommit_task_barrier_.NotifyOne();
-    }
-
-    MarkingDeque* marking_deque_;
-    DISALLOW_COPY_AND_ASSIGN(UncommitTask);
-  };
-
   static const size_t kMaxSize = 4 * MB;
   static const size_t kMinSize = 256 * KB;
 
-  // Must be called with mutex lock.
   void EnsureCommitted();
-
-  // Must be called with mutex lock.
   void Uncommit();
 
-  // Must be called with mutex lock.
-  void StartUncommitTask();
-
-  void CancelOrWaitForUncommitTask();
-
-  base::Mutex mutex_;
-  base::ConditionVariable uncommit_task_barrier_;
-
   base::VirtualMemory* backing_store_;
   size_t backing_store_committed_size_;
   HeapObject** array_;
@@ -178,12 +138,7 @@ class MarkingDeque {
   int bottom_;
   int mask_;
   bool overflowed_;
-  // in_use_ == true after taking mutex lock implies that the marking deque is
-  // committed and will stay committed at least until in_use_ == false.
   bool in_use_;
-  bool uncommit_task_pending_;
-  uint32_t uncommit_task_id_;
-  Heap* heap_;
 
   DISALLOW_COPY_AND_ASSIGN(MarkingDeque);
 };

test/cctest/heap/test-mark-compact.cc

@@ -51,7 +51,7 @@ using v8::Just;
 
 TEST(MarkingDeque) {
   CcTest::InitializeVM();
-  MarkingDeque s(CcTest::i_isolate()->heap());
+  MarkingDeque s;
   s.SetUp();
   s.StartUsing();
   Address original_address = reinterpret_cast<Address>(&s);