Delay deallocation of default actors when they're currently running.

For ordinary memory-management reasons, this should only ever
happen when there will be no more uses of the actor outside of the
actor runtime.  The actor runtime, meanwhile, doesn't care about
anything except the default-actor control state of the actor.  So
we can just allow the rest of the actor to be destructed when it
isn't needed anymore, then destroy the actor state and deallocate
the object when we get around to switching off the executor.

This does assume that the task doesn't do anything which semantically
detects the executor it's on before switching off it, since doing so
might read a bogus executor.  However, we should only get an executor
in a zombie state like this when a hop has been removed or reordered,
and detection events should count as inhibiting that and forcing the
true executor to be switched to (and thus detected).

(But maybe lifetime optimization can make this happen?  Maybe we
need semantic detection to filter out zombie executors.)

Author: rjmccall

include/swift/Runtime/Concurrency.h

@@ -512,6 +512,14 @@ void swift_defaultActor_initialize(DefaultActor *actor);
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_defaultActor_destroy(DefaultActor *actor);
 
+/// Deallocate an instance of a default actor.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+void swift_defaultActor_deallocate(DefaultActor *actor);
+
+/// Deallocate an instance of what might be a default actor.
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+void swift_defaultActor_deallocateResilient(HeapObject *actor);
+
 /// Enqueue a job on the default actor implementation.
 ///
 /// The job must be ready to run.  Notably, if it's a task, that

include/swift/Runtime/RuntimeFunctions.def

@@ -1656,6 +1656,22 @@ FUNCTION(DefaultActorDestroy,
          ARGS(RefCountedPtrTy),
          ATTRS(NoUnwind))
 
+// void swift_defaultActor_deallocate(DefaultActor *actor);
+FUNCTION(DefaultActorDeallocate,
+         swift_defaultActor_deallocate, SwiftCC,
+         ConcurrencyAvailability,
+         RETURNS(VoidTy),
+         ARGS(RefCountedPtrTy),
+         ATTRS(NoUnwind))
+
+// void swift_defaultActor_deallocateResilient(HeapObject *actor);
+FUNCTION(DefaultActorDeallocateResilient,
+         swift_defaultActor_deallocateResilient, SwiftCC,
+         ConcurrencyAvailability,
+         RETURNS(VoidTy),
+         ARGS(RefCountedPtrTy),
+         ATTRS(NoUnwind))
+
 //// void swift_taskGroup_initialize(AsyncTask *task, TaskGroup *group);
 //FUNCTION(TaskGroupInitialize,
 //         swift_taskGroup_initialize, SwiftCC,

lib/IRGen/GenClass.cpp

@@ -876,15 +876,47 @@ static void getInstanceSizeAndAlignMask(IRGenFunction &IGF,
     = emitClassResilientInstanceSizeAndAlignMask(IGF, selfClass, metadata);
 }
 
+static llvm::Value *emitCastToHeapObject(IRGenFunction &IGF,
+                                         llvm::Value *value) {
+  return IGF.Builder.CreateBitCast(value, IGF.IGM.RefCountedPtrTy);
+}
+
 void irgen::emitClassDeallocation(IRGenFunction &IGF, SILType selfType,
                                   llvm::Value *selfValue) {
   auto *theClass = selfType.getClassOrBoundGenericClass();
 
+  // We want to deallocate default actors or potential default
+  // actors differently.  We assume that being a default actor
+  // is purely a property of the root actor class, so just go to
+  // that class.
+  if (auto rootActorClass = theClass->getRootActorClass()) {
+    // If it's a default actor, use swift_deallocDefaultActor.
+    if (rootActorClass->isDefaultActor(IGF.IGM.getSwiftModule(),
+                                       ResilienceExpansion::Maximal)) {
+      selfValue = emitCastToHeapObject(IGF, selfValue);
+      IGF.Builder.CreateCall(IGF.IGM.getDefaultActorDeallocateFn(),
+                             {selfValue});
+      return;
+    }
+
+    // If it's possibly a default actor, use a resilient pattern.
+    if (!rootActorClass->isForeign() &&
+        rootActorClass->isResilient(IGF.IGM.getSwiftModule(),
+                                    ResilienceExpansion::Maximal)) {
+      selfValue = emitCastToHeapObject(IGF, selfValue);
+      IGF.Builder.CreateCall(IGF.IGM.getDefaultActorDeallocateResilientFn(),
+                             {selfValue});
+      return;
+    }
+
+    // Otherwise use the normal path.
+  }
+
   llvm::Value *size, *alignMask;
   getInstanceSizeAndAlignMask(IGF, selfType, theClass, selfValue,
                               size, alignMask);
 
-  selfValue = IGF.Builder.CreateBitCast(selfValue, IGF.IGM.RefCountedPtrTy);
+  selfValue = emitCastToHeapObject(IGF, selfValue);
   emitDeallocateClassInstance(IGF, selfValue, size, alignMask);
 }
 

stdlib/public/Concurrency/Actor.cpp

@@ -399,15 +399,28 @@ class DefaultActorImpl : public HeapObject {
 
     /// There is currently a thread processing the actor at the stored
     /// max priority.
-    Running
+    Running,
+
+    /// The actor is a zombie that's been fully released but is still
+    /// running.  We delay deallocation until its running thread gives
+    /// it up, which fortunately doesn't touch anything in the
+    /// actor except for the DefaultActorImpl.
+    ///
+    /// To coordinate between the releasing thread and the running
+    /// thread, we have a two-stage "latch".  This is because the
+    /// releasing thread does not atomically decide to not deallocate.
+    /// Fortunately almost all of the overhead here is only incurred
+    /// when we actually do end up in the zombie state.
+    Zombie_Latching,
+    Zombie_ReadyForDeallocation
   };
 
   struct Flags : public FlagSet<size_t> {
     enum : size_t {
       Status_offset = 0,
-      Status_width = 2,
+      Status_width = 3,
 
-      HasActiveInlineJob = 2,
+      HasActiveInlineJob = 3,
 
       MaxPriority = 8,
       MaxPriority_width = JobFlags::Priority_width,
@@ -420,6 +433,19 @@ class DefaultActorImpl : public HeapObject {
     FLAGSET_DEFINE_FIELD_ACCESSORS(Status_offset, Status_width, Status,
                                    getStatus, setStatus)
 
+    bool isAnyRunningStatus() const {
+      auto status = getStatus();
+      return status == Status::Running ||
+             status == Status::Zombie_Latching ||
+             status == Status::Zombie_ReadyForDeallocation;
+    }
+
+    bool isAnyZombieStatus() const {
+      auto status = getStatus();
+      return status == Status::Zombie_Latching ||
+             status == Status::Zombie_ReadyForDeallocation;
+    }
+
     /// Is there currently an active processing job allocated inline
     /// in the actor?
     FLAGSET_DEFINE_FLAG_ACCESSORS(HasActiveInlineJob,
@@ -459,10 +485,12 @@ class DefaultActorImpl : public HeapObject {
   }
 
   /// Properly destruct an actor, except for the heap header.
-  void destroy() {
-    assert(CurrentState.load(std::memory_order_relaxed).Flags.getStatus()
-             == Status::Idle && "actor not idle during destruction?");
-  }
+  void destroy();
+
+  /// Properly respond to the last release of a default actor.  Note
+  /// that the actor will have been completely torn down by the time
+  /// we reach this point.
+  void deallocate();
 
   /// Add a job to this actor.
   void enqueue(Job *job);
@@ -477,6 +505,8 @@ class DefaultActorImpl : public HeapObject {
   Job *claimNextJobOrGiveUp(bool actorIsOwned, RunningJobInfo runner);
 
 private:
+  void deallocateUnconditional();
+
   /// Schedule an inline processing job.  This can generally only be
   /// done if we know nobody else is trying to do it at the same time,
   /// e.g. if this thread just sucessfully transitioned the actor from
@@ -531,6 +561,53 @@ static DefaultActor *asAbstract(DefaultActorImpl *actor) {
 /*********************** DEFAULT ACTOR IMPLEMENTATION ************************/
 /*****************************************************************************/
 
+void DefaultActorImpl::destroy() {
+  auto oldState = CurrentState.load(std::memory_order_relaxed);
+  while (true) {
+    assert(!oldState.FirstJob && "actor has queued jobs at destruction");
+    if (oldState.Flags.getStatus() == Status::Idle) return;
+
+    assert(oldState.Flags.getStatus() == Status::Running &&
+           "actor scheduled but not running at destruction");
+
+    // If the actor is currently running, set it to zombie status
+    // so that we know to deallocate it when it's given up.
+    auto newState = oldState;
+    newState.Flags.setStatus(Status::Zombie_Latching);
+    if (CurrentState.compare_exchange_weak(oldState, newState,
+                                           std::memory_order_relaxed,
+                                           std::memory_order_relaxed))
+      return;
+  }
+}
+
+void DefaultActorImpl::deallocate() {
+  // If we're in a zombie state waiting to latch, put the actor in the
+  // ready-for-deallocation state, but don't actually deallocate yet.
+  // Note that we should never see the actor already in the
+  // ready-for-deallocation state; giving up the actor while in the
+  // latching state will always put it in Idle state.
+  auto oldState = CurrentState.load(std::memory_order_relaxed);
+  while (oldState.Flags.getStatus() == Status::Zombie_Latching) {
+    auto newState = oldState;
+    newState.Flags.setStatus(Status::Zombie_ReadyForDeallocation);
+    if (CurrentState.compare_exchange_weak(oldState, newState,
+                                           std::memory_order_relaxed,
+                                           std::memory_order_relaxed))
+      return;
+  }
+
+  assert(oldState.Flags.getStatus() == Status::Idle);
+
+  deallocateUnconditional();
+}
+
+void DefaultActorImpl::deallocateUnconditional() {
+  auto metadata = cast<ClassMetadata>(this->metadata);
+  swift_deallocObject(this, metadata->getInstanceSize(),
+                      metadata->getInstanceAlignMask());
+}
+
 /// Given that a job is enqueued normally on a default actor, get/set
 /// the next job in the actor's queue.
 ///
@@ -888,17 +965,30 @@ void DefaultActorImpl::giveUpThread(RunningJobInfo runner) {
   fprintf(stderr, "[%p] giving up thread for actor %p\n", pthread_self(), this);
 #endif
   auto oldState = CurrentState.load(std::memory_order_acquire);
-  assert(oldState.Flags.getStatus() == Status::Running);
+  assert(oldState.Flags.isAnyRunningStatus());
 
   ProcessOverrideJob *overridesToWake = nullptr;
   auto firstNewJob = preprocessQueue(oldState.FirstJob, JobRef(), nullptr,
                                      overridesToWake);
 
   _swift_tsan_release(this);
   while (true) {
+    // In Zombie_ReadyForDeallocation state, nothing else should
+    // be touching the atomic, and there's no point updating it.
+    if (oldState.Flags.getStatus() == Status::Zombie_ReadyForDeallocation) {
+      wakeOverrides(overridesToWake, oldState.Flags.getMaxPriority());
+      deallocateUnconditional();
+      return;
+    }
+
+    // In Zombie_Latching state, we should try to update to Idle;
+    // if we beat the releasing thread, it'll deallocate.
+    // In Running state, we may need to schedule a processing job.
+
     State newState = oldState;
     newState.FirstJob = JobRef::getPreprocessed(firstNewJob);
     if (firstNewJob) {
+      assert(oldState.Flags.getStatus() == Status::Running);
       newState.Flags.setStatus(Status::Scheduled);
     } else {
       newState.Flags.setStatus(Status::Idle);
@@ -946,7 +1036,7 @@ void DefaultActorImpl::giveUpThread(RunningJobInfo runner) {
     // The priority of the remaining work.
     auto newPriority = newState.Flags.getMaxPriority();
 
-    // Wake any overrides.
+    // Process the override commands we found.
     wakeOverrides(overridesToWake, newPriority);
 
     // This is the actor's owning job; per the ownership rules (see
@@ -983,13 +1073,16 @@ Job *DefaultActorImpl::claimNextJobOrGiveUp(bool actorIsOwned,
 
   // The status had better be Running unless we're trying to acquire
   // our first job.
-  assert(oldState.Flags.getStatus() == Status::Running ||
-         !actorIsOwned);
+  assert(oldState.Flags.isAnyRunningStatus() || !actorIsOwned);
 
   // If we don't yet own the actor, we need to try to claim the actor
   // first; we cannot safely access the queue memory yet because other
   // threads may concurrently be trying to do this.
   if (!actorIsOwned) {
+    // We really shouldn't ever be in a state where we're trying to take
+    // over a non-running actor if the actor is in a zombie state.
+    assert(!oldState.Flags.isAnyZombieStatus());
+
     while (true) {
       // A helper function when the only change we need to try is to
       // update for an inline runner.
@@ -1061,7 +1154,7 @@ Job *DefaultActorImpl::claimNextJobOrGiveUp(bool actorIsOwned,
     }
   }
 
-  assert(oldState.Flags.getStatus() == Status::Running);
+  assert(oldState.Flags.isAnyRunningStatus());
 
   // We should have taken care of the inline-job bookkeeping now.
   assert(!oldState.Flags.hasActiveInlineJob() || !runner.wasInlineJob());
@@ -1075,6 +1168,14 @@ Job *DefaultActorImpl::claimNextJobOrGiveUp(bool actorIsOwned,
   Optional<JobPriority> remainingJobPriority;
   _swift_tsan_release(this);
   while (true) {
+    // In Zombie_ReadyForDeallocation state, nothing else should
+    // be touching the atomic, and there's no point updating it.
+    if (oldState.Flags.getStatus() == Status::Zombie_ReadyForDeallocation) {
+      wakeOverrides(overridesToWake, oldState.Flags.getMaxPriority());
+      deallocateUnconditional();
+      return nullptr;
+    }
+
     State newState = oldState;
 
     // If the priority we're currently running with is adqeuate for
@@ -1300,6 +1401,8 @@ void DefaultActorImpl::enqueue(Job *job) {
   OverrideJobCache overrideJob;
 
   while (true) {
+    assert(!oldState.Flags.isAnyZombieStatus() &&
+           "enqueuing work on a zombie actor");
     auto newState = oldState;
 
     // Put the job at the front of the job list (which will get
@@ -1387,6 +1490,9 @@ bool DefaultActorImpl::tryAssumeThread(RunningJobInfo runner) {
     }
   }
 
+  assert(!oldState.Flags.isAnyZombieStatus() &&
+         "trying to assume a zombie actor");
+
   return false;
 }
 
@@ -1402,6 +1508,34 @@ void swift::swift_defaultActor_enqueue(Job *job, DefaultActor *_actor) {
   asImpl(_actor)->enqueue(job);
 }
 
+void swift::swift_defaultActor_deallocate(DefaultActor *_actor) {
+  asImpl(_actor)->deallocate();
+}
+
+static bool isDefaultActorClass(const ClassMetadata *metadata) {
+  assert(metadata->isTypeMetadata());
+  while (true) {
+    // Trust the class descriptor if it says it's a default actor.
+    if (metadata->getDescription()->isDefaultActor())
+      return true;
+
+    // Go to the superclass.
+    metadata = metadata->Superclass;
+
+    // If we run out of Swift classes, it's not a default actor.
+    if (!metadata || !metadata->isTypeMetadata()) return false;
+  }
+}
+
+void swift::swift_defaultActor_deallocateResilient(HeapObject *actor) {
+  auto metadata = cast<ClassMetadata>(actor->metadata);
+  if (isDefaultActorClass(metadata))
+    return swift_defaultActor_deallocate(static_cast<DefaultActor*>(actor));
+
+  swift_deallocObject(actor, metadata->getInstanceSize(),
+                      metadata->getInstanceAlignMask());
+}
+
 /// FIXME: only exists for the quick-and-dirty MainActor implementation.
 namespace swift {
   Metadata* MainActorMetadata = nullptr;

test/IRGen/async/default_actor.swift

@@ -23,24 +23,24 @@ import resilient_actor
 
 // CHECK-LABEL: define hidden swiftcc void @"$s13default_actor1ACfD"(%T13default_actor1AC* swiftself %0)
 // CHECK-NOT: ret void
-// CHECK:     call swiftcc void @swift_deallocObject(
+// CHECK:     call swiftcc void @swift_defaultActor_deallocate(
 // CHECK:     ret void
 actor A {}
 
 // CHECK-LABEL: define hidden swiftcc void @"$s13default_actor1BCfD"(%T13default_actor1BC* swiftself %0)
 // CHECK-NOT: ret void
-// CHECK:     call swiftcc void @swift_deallocObject(
+// CHECK:     call swiftcc void @swift_defaultActor_deallocateResilient(
 // CHECK:     ret void
 actor B : ResilientBaseActor {}
 
 // CHECK-LABEL: define hidden swiftcc void @"$s13default_actor1CCfD"(%T13default_actor1CC* swiftself %0)
 // CHECK-NOT: ret void
-// CHECK:     call swiftcc void @swift_deallocObject(
+// CHECK:     call swiftcc void @swift_defaultActor_deallocateResilient(
 // CHECK:     ret void
 actor C : FixedSubclassOfResilientBaseActor {}
 
 // CHECK-LABEL: define hidden swiftcc void @"$s13default_actor1DCfD"(%T13default_actor1DC* swiftself %0)
 // CHECK-NOT: ret void
-// CHECK:     call swiftcc void @swift_deallocObject(
+// CHECK:     call swiftcc void @swift_defaultActor_deallocate(
 // CHECK:     ret void
 actor D : FixedBaseActor {}