Applied review comments

Author: nickolas-pohilets

include/swift/Basic/PriorityQueue.h

@@ -89,23 +89,25 @@ class PriorityQueue {
 
   /// Add a chain of nodes of mixed priorities to this queue.
   void enqueueContentsOf(Node otherHead) {
+    if (!otherHead) return;
     Node runHead = otherHead;
-    while (runHead) {
-      int priorityIndex = NodeTraits::getPriorityIndex(runHead);
-
+    int priorityIndex = NodeTraits::getPriorityIndex(runHead);
+    do {
       // Find run of jobs of the same priority
       Node runTail = runHead;
       Node next = NodeTraits::getNext(runTail);
-      while (true) {
-        if (!next) break;
-        if (NodeTraits::getPriorityIndex(next) != priorityIndex) break;
+      int nextRunPriorityIndex;
+      while (next) {
+        nextRunPriorityIndex = NodeTraits::getPriorityIndex(next);
+        if (nextRunPriorityIndex != priorityIndex) break;
         runTail = next;
         next = NodeTraits::getNext(runTail);
       }
 
       enqueueRun(priorityIndex, runHead, runTail);
       runHead = next;
-    }
+      priorityIndex = nextRunPriorityIndex;
+    } while(runHead);
   }
 
   Node dequeue() {

stdlib/public/Concurrency/Actor.cpp

@@ -969,8 +969,7 @@ class DefaultActorImplHeader : public HeapObject {
 // All the fields accessed under the actor's lock should be moved
 // to the end of the default-actor reservation to minimize false sharing.
 // The memory following the DefaultActorImpl object are the stored properties of
-// the actor, which are likely all isolated and therefore used only by the
-// current processing thread.
+// the actor, which are all accessed only by the current processing thread.
 class DefaultActorImplFooter {
 protected:
 #if !SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
@@ -980,10 +979,11 @@ class DefaultActorImplFooter {
   // stored inside ActiveActorStatus. This list contains jobs in the LIFO order
   // regardless of their priorities.
   //
-  // After that processing thread collects them and adds to the
-  // `prioritisedJobs` while holding the actor lock.
+  // When the processing thread sees new incoming jobs in
+  // ActiveActorStatus, it reverses them and inserts them into
+  // prioritizedJobs in the appropriate priority bucket.
   //
-  PriorityQueue prioritisedJobs;
+  PriorityQueue prioritizedJobs;
 
   // As an optimisation, we handle unprioritized jobs when obtaining actor lock.
   // This flag is used to skip `processIncomingQueue()` on the iteration of the
@@ -1056,7 +1056,7 @@ class DefaultActorImpl
     new (&this->drainLock) Mutex();
 #else
    _status().store(ActiveActorStatus(), std::memory_order_relaxed);
-   new (&this->prioritisedJobs) PriorityQueue();
+   new (&this->prioritizedJobs) PriorityQueue();
    this->shouldProcessIncomingQueue = true;
 #endif
     SWIFT_TASK_DEBUG_LOG("Creating default actor %p", this);
@@ -1128,7 +1128,7 @@ class DefaultActorImpl
   /// Called with actor lock held on current thread.
   void processIncomingQueue();
 
-  /// Processes claimed incoming jobs into `prioritisedJobs`.
+  /// Processes claimed incoming jobs into `prioritizedJobs`.
   /// Incoming jobs are of mixed priorities and in LIFO order.
   /// Called with actor lock held on current thread.
   void handleUnprioritizedJobs(Job *head);
@@ -1426,7 +1426,7 @@ void DefaultActorImpl::handleUnprioritizedJobs(Job *head) {
     reversed = head;
     head = next;
   }
-  prioritisedJobs.enqueueContentsOf(reversed);
+  prioritizedJobs.enqueueContentsOf(reversed);
   shouldProcessIncomingQueue = false;
 }
 
@@ -1435,8 +1435,8 @@ Job *DefaultActorImpl::drainOne() {
   SWIFT_TASK_DEBUG_LOG("Draining one job from default actor %p", this);
 
   processIncomingQueue();
-  traceJobQueue(this, prioritisedJobs.peek());
-  auto firstJob = prioritisedJobs.dequeue();
+  traceJobQueue(this, prioritizedJobs.peek());
+  auto firstJob = prioritizedJobs.dequeue();
   if (!firstJob) {
     SWIFT_TASK_DEBUG_LOG("No jobs to drain on actor %p", this);
   } else {
@@ -1569,11 +1569,11 @@ void DefaultActorImpl::destroy() {
   assert(!oldState.getFirstUnprioritisedJob() && "actor has queued jobs at destruction");
 
   if (oldState.isIdle()) {
-    assert(prioritisedJobs.empty() && "actor has queued jobs at destruction");
+    assert(prioritizedJobs.empty() && "actor has queued jobs at destruction");
     return;
   }
   assert(oldState.isRunning() && "actor scheduled but not running at destruction");
-  // In running state we cannot safely access prioritisedJobs to assert that it is empty.
+  // In running state we cannot safely access prioritizedJobs to assert that it is empty.
 #endif
 }
 
@@ -1667,7 +1667,7 @@ retry:;
 
       assert(oldState.getMaxPriority() == JobPriority::Unspecified);
       assert(!oldState.getFirstUnprioritisedJob());
-      // We cannot assert here that prioritisedJobs is empty,
+      // We cannot assert here that prioritizedJobs is empty,
       // because lock is not held yet. Raise a flag to assert after getting the lock.
       assertNoJobs = true;
     }
@@ -1687,7 +1687,7 @@ retry:;
                                  std::memory_order_relaxed)) {
       _swift_tsan_acquire(this);
       if (assertNoJobs) {
-        assert(prioritisedJobs.empty());
+        assert(prioritizedJobs.empty());
       }
       traceActorStateTransition(this, oldState, newState, distributedActorIsRemote);
       handleUnprioritizedJobs(oldState.getFirstUnprioritisedJob());
@@ -1738,8 +1738,8 @@ bool DefaultActorImpl::unlock(bool forceUnlock)
     }
 
     auto newState = oldState;
-    // Lock is still held at this point, so it is safe to access prioritisedJobs
-    if (!prioritisedJobs.empty() || oldState.getFirstUnprioritisedJob()) {
+    // Lock is still held at this point, so it is safe to access prioritizedJobs
+    if (!prioritizedJobs.empty() || oldState.getFirstUnprioritisedJob()) {
       // There is work left to do, don't unlock the actor
       if (!forceUnlock) {
         SWIFT_TASK_DEBUG_LOG("Unlock-ing actor %p failed", this);