Changeset 226725 in webkit for trunk/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp

Timestamp:

Jan 10, 2018, 11:41:12 AM (8 years ago)

Author:

[email protected]

Message:

Unreviewed, rolling out r226667 and r226673.
​https://bugs.webkit.org/show_bug.cgi?id=181488

This caused a flaky crash. (Requested by mlewis13 on #webkit).

Reverted changesets:

"CodeBlocks should be in IsoSubspaces"
​https://bugs.webkit.org/show_bug.cgi?id=180884
​https://trac.webkit.org/changeset/226667

"REGRESSION (r226667): CodeBlocks should be in IsoSubspaces"
​https://bugs.webkit.org/show_bug.cgi?id=180884
​https://trac.webkit.org/changeset/226673

File:

• trunk/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp (modified) (8 diffs)

trunk/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp

@@ -79 +79 @@
     RELEASE_ASSERT(!m_numThreadsThatMayProduceWork);
         
+    for (unsigned indexToRun : m_didExecuteInParallel)
+        m_set.m_set[indexToRun]->finishParallelWork(m_mainVisitor);
+    m_didExecuteInParallel.clear();
+    
     if (!m_toExecuteSequentially.isEmpty()) {
         for (unsigned indexToRun : m_toExecuteSequentially)
@@ -86 +90 @@
         
     RELEASE_ASSERT(m_toExecuteInParallel.isEmpty());
+    RELEASE_ASSERT(!m_toExecuteInParallelSet.bitCount());
 }
 
@@ -152 +157 @@
     
     constraint.prepareToExecute(NoLockingNecessary, m_mainVisitor);
-    constraint.execute(m_mainVisitor);
-    m_executed.set(constraint.index());
-}
-
-void MarkingConstraintSolver::addParallelTask(RefPtr<SharedTask<void(SlotVisitor&)>> task, MarkingConstraint& constraint)
-{
-    auto locker = holdLock(m_lock);
-    m_toExecuteInParallel.append(TaskWithConstraint(WTFMove(task), &constraint));
+    ConstraintParallelism parallelism = constraint.execute(m_mainVisitor);
+    didExecute(parallelism, constraint.index());
 }
 
@@ -166 +165 @@
     for (;;) {
         bool doParallelWorkMode;
-        MarkingConstraint* constraint = nullptr;
-        unsigned indexToRun = UINT_MAX;
-        TaskWithConstraint task;
+        unsigned indexToRun;
         {
             auto locker = holdLock(m_lock);
@@ -177 +174 @@
                         return false;
                     
-                    task = m_toExecuteInParallel.first();
-                    constraint = task.constraint;
+                    indexToRun = m_toExecuteInParallel.first();
                     doParallelWorkMode = true;
                     return true;
                 };
-                
+                            
                 auto tryNextConstraint = [&] () -> bool {
                     if (!m_pickNextIsStillActive)
@@ -197 +193 @@
                             continue;
                                     
-                        MarkingConstraint& candidateConstraint = *m_set.m_set[*pickResult];
-                        if (candidateConstraint.concurrency() == ConstraintConcurrency::Sequential) {
+                        MarkingConstraint& constraint = *m_set.m_set[*pickResult];
+                        if (constraint.concurrency() == ConstraintConcurrency::Sequential) {
                             m_toExecuteSequentially.append(*pickResult);
                             continue;
                         }
-                        if (candidateConstraint.parallelism() == ConstraintParallelism::Parallel)
+                        if (constraint.parallelism() == ConstraintParallelism::Parallel)
                             m_numThreadsThatMayProduceWork++;
                         indexToRun = *pickResult;
-                        constraint = &candidateConstraint;
                         doParallelWorkMode = false;
-                        constraint->prepareToExecute(locker, visitor);
+                        constraint.prepareToExecute(locker, visitor);
                         return true;
                     }
@@ -232 +227 @@
         }
                     
+        ConstraintParallelism parallelism = ConstraintParallelism::Sequential;
+                    
+        MarkingConstraint& constraint = *m_set.m_set[indexToRun];
+                    
         if (doParallelWorkMode)
-            constraint->doParallelWork(visitor, *task.task);
-        else {
-            if (constraint->parallelism() == ConstraintParallelism::Parallel) {
-                visitor.m_currentConstraint = constraint;
-                visitor.m_currentSolver = this;
-            }
-            
-            constraint->execute(visitor);
-            
-            visitor.m_currentConstraint = nullptr;
-            visitor.m_currentSolver = nullptr;
-        }
-        
+            constraint.doParallelWork(visitor);
+        else
+            parallelism = constraint.execute(visitor);
+                    
         {
             auto locker = holdLock(m_lock);
-            
+                        
             if (doParallelWorkMode) {
-                if (!m_toExecuteInParallel.isEmpty()
-                    && task == m_toExecuteInParallel.first())
-                    m_toExecuteInParallel.takeFirst();
-                else
-                    ASSERT(!m_toExecuteInParallel.contains(task));
+                if (m_toExecuteInParallelSet.get(indexToRun)) {
+                    m_didExecuteInParallel.append(indexToRun);
+                                
+                    m_toExecuteInParallel.takeFirst(
+                        [&] (unsigned value) { return value == indexToRun; });
+                    m_toExecuteInParallelSet.clear(indexToRun);
+                }
             } else {
-                if (constraint->parallelism() == ConstraintParallelism::Parallel)
+                if (constraint.parallelism() == ConstraintParallelism::Parallel)
                     m_numThreadsThatMayProduceWork--;
                 m_executed.set(indexToRun);
+                if (parallelism == ConstraintParallelism::Parallel) {
+                    m_toExecuteInParallel.append(indexToRun);
+                    m_toExecuteInParallelSet.set(indexToRun);
+                }
             }
                         
@@ -266 +262 @@
 }
 
+void MarkingConstraintSolver::didExecute(ConstraintParallelism parallelism, unsigned index)
+{
+    m_executed.set(index);
+    if (parallelism == ConstraintParallelism::Parallel) {
+        m_toExecuteInParallel.append(index);
+        m_toExecuteInParallelSet.set(index);
+    }
+}
+
 } // namespace JSC