Moved prioritisedJobs.enqueue the end of the layout of the DefaultActorImpl to minimise false sharing

Author: nickolas-pohilets

stdlib/public/Concurrency/Actor.cpp

@@ -998,44 +998,65 @@ static_assert(sizeof(ActiveActorStatus) == ACTIVE_ACTOR_STATUS_SIZE,
 /// exist yet. As a result, the subset of rules that currently apply
 /// are (1), (3), (5), (6).
 class DefaultActorImpl : public HeapObject {
+  struct Header {
 #if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
-  // If actors are locks, we don't need to maintain any extra bookkeeping in the
-  // ActiveActorStatus since all threads which are contending will block
-  // synchronously, no job queue is needed and the lock will handle all priority
-  // escalation logic
-  Mutex drainLock;
+    // If actors are locks, we don't need to maintain any extra bookkeeping in the
+    // ActiveActorStatus since all threads which are contending will block
+    // synchronously, no job queue is needed and the lock will handle all priority
+    // escalation logic
+    Mutex drainLock;
 #else
-  // Note: There is some padding that is added here by the compiler in order to
-  // enforce alignment. This is space that is available for us to use in
-  // the future
-  alignas(sizeof(ActiveActorStatus)) char StatusStorage[sizeof(ActiveActorStatus)];
+    // Note: There is some padding that is added here by the compiler in order to
+    // enforce alignment. This is space that is available for us to use in
+    // the future
+    alignas(sizeof(ActiveActorStatus)) char StatusStorage[sizeof(ActiveActorStatus)];
+#endif
+    // TODO (rokhinip): Make this a flagset
+    bool isDistributedRemoteActor;
+  } header;
 
+#if !SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
   using SimpleQueue = swift::SimpleQueue<Job *, JobQueueTraits>;
   using PriorityQueue = swift::PriorityQueue<Job *, JobQueueTraits>;
 
-  // When enqueued, jobs are atomically added to a linked list with the head
-  // 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.
-  //
-  // TODO: Move this to the end of the actor memory to minimise false sharing
-  //
-  PriorityQueue prioritisedJobs;
+  struct Footer {
+    // When enqueued, jobs are atomically added to a linked list with the head
+    // 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.
+    //
+    PriorityQueue prioritisedJobs;
+  };
+
+  struct Padding {
+    enum {
+      // Available space
+      x1 = (int)(sizeof(void *) * NumWords_DefaultActor),
+      // ... subtracting size of footer
+      x2 = x1 - (int)sizeof(Footer),
+      // ... ensuring footer is aligned
+      x3 = x2 - (int)(x2 % alignof(Footer)),
+      // ... subtracting size of header
+      x4 = x3 - (int)sizeof(Header),
+      // ... subtracting padding before header
+      size = x4 - (int)((alignof(Header) - sizeof(HeapObject) % alignof(Header)) % alignof(Header))
+    };
+  };
+  char padding[Padding::size];
+  Footer footer;
 #endif
-  // TODO (rokhinip): Make this a flagset
-  bool isDistributedRemoteActor;
 
 public:
   /// Properly construct an actor, except for the heap header.
   void initialize(bool isDistributedRemote = false) {
-    this->isDistributedRemoteActor = isDistributedRemote;
+    this->header.isDistributedRemoteActor = isDistributedRemote;
 #if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
-    new (&this->drainLock) Mutex();
+    new (&this->header.drainLock) Mutex();
 #else
    _status().store(ActiveActorStatus(), std::memory_order_relaxed);
-   new (&this->prioritisedJobs) PriorityQueue();
+   new (&this->footer.prioritisedJobs) PriorityQueue();
 #endif
     SWIFT_TASK_DEBUG_LOG("Creating default actor %p", this);
     concurrency::trace::actor_create(this);
@@ -1075,20 +1096,25 @@ class DefaultActorImpl : public HeapObject {
 
 #if !SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
   swift::atomic<ActiveActorStatus> &_status() {
-    return reinterpret_cast<swift::atomic<ActiveActorStatus>&> (this->StatusStorage);
+    return reinterpret_cast<swift::atomic<ActiveActorStatus>&> (this->header.StatusStorage);
   }
 
   const swift::atomic<ActiveActorStatus> &_status() const {
-    return reinterpret_cast<const swift::atomic<ActiveActorStatus>&> (this->StatusStorage);
+    return reinterpret_cast<const swift::atomic<ActiveActorStatus>&> (this->header.StatusStorage);
   }
 
   // Only for static assert use below, not for actual use otherwise
   static constexpr size_t offsetOfActiveActorStatus() {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Winvalid-offsetof"
-    return offsetof(DefaultActorImpl, StatusStorage);
+    return offsetof(DefaultActorImpl, header.StatusStorage);
 #pragma clang diagnostic pop
   }
+
+  PriorityQueue &_prioritisedJobs() {
+    return footer.prioritisedJobs;
+  }
+
 #endif /* !SWIFT_CONCURRENCY_ACTORS_AS_LOCKS */
 
 private:
@@ -1417,7 +1443,7 @@ void DefaultActorImpl::processJobs() {
   auto job = jobs.head;
   while (job) {
     auto next = getNextJob(job);
-    prioritisedJobs.enqueue(job);
+    _prioritisedJobs().enqueue(job);
     job = next;
   }
 }
@@ -1427,8 +1453,8 @@ Job *DefaultActorImpl::drainOne() {
   SWIFT_TASK_DEBUG_LOG("Draining one job from default actor %p", this);
 
   processJobs();
-  traceJobQueue(this, prioritisedJobs.peek());
-  auto firstJob = prioritisedJobs.dequeue();
+  traceJobQueue(this, _prioritisedJobs().peek());
+  auto firstJob = _prioritisedJobs().dequeue();
   if (!firstJob) {
     SWIFT_TASK_DEBUG_LOG("No jobs to drain on actor %p", this);
   } else {
@@ -1673,7 +1699,7 @@ retry:;
                                  std::memory_order_relaxed)) {
       _swift_tsan_acquire(this);
       if (assertNoJobs) {
-        assert(prioritisedJobs.empty());
+        assert(_prioritisedJobs().empty());
       }
       traceActorStateTransition(this, oldState, newState, distributedActorIsRemote);
       return true;
@@ -1724,7 +1750,7 @@ 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()) {
+    if (!_prioritisedJobs().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);
@@ -2245,5 +2271,5 @@ bool swift::swift_distributed_actor_is_remote(HeapObject *_actor) {
 }
 
 bool DefaultActorImpl::isDistributedRemote() {
-  return this->isDistributedRemoteActor;
+  return this->header.isDistributedRemoteActor;
 }