Skip to content

Commit 2c2aa05

Browse files
committed
Remove the inline job in actors and make them use OOL jobs always. This is because we need 16 bytes of atomic state in the actor which is not available to us on arm64_32 since the inline job takes up 40 of the 48 bytes for use.
Enable priority escalation support on arm64_32. Radar-Id: rdar://problem/90725051
1 parent c36716e commit 2c2aa05

File tree

1 file changed

+21
-96
lines changed

1 file changed

+21
-96
lines changed

stdlib/public/Concurrency/Actor.cpp

Lines changed: 21 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -422,20 +422,6 @@ namespace {
422422

423423
class DefaultActorImpl;
424424

425-
/// A job to process a default actor. Allocated inline in the actor.
426-
class ProcessInlineJob : public Job {
427-
public:
428-
ProcessInlineJob(JobPriority priority)
429-
: Job({JobKind::DefaultActorInline, priority}, &process) {}
430-
431-
SWIFT_CC(swiftasync)
432-
static void process(Job *job);
433-
434-
static bool classof(const Job *job) {
435-
return job->Flags.getKind() == JobKind::DefaultActorInline;
436-
}
437-
};
438-
439425
/// A job to process a default actor that's allocated separately from
440426
/// the actor.
441427
class ProcessOutOfLineJob : public Job {
@@ -453,25 +439,6 @@ class ProcessOutOfLineJob : public Job {
453439
}
454440
};
455441

456-
/// Information about the currently-running processing job.
457-
struct RunningJobInfo {
458-
enum KindType : uint8_t {
459-
Inline, Other
460-
};
461-
KindType Kind;
462-
463-
bool wasInlineJob() const {
464-
return Kind == Inline;
465-
}
466-
467-
static RunningJobInfo forOther() {
468-
return {Other};
469-
}
470-
static RunningJobInfo forInline() {
471-
return {Inline};
472-
}
473-
};
474-
475442
class JobRef {
476443
enum : uintptr_t {
477444
NeedsPreprocessing = 0x1,
@@ -532,12 +499,6 @@ class JobRef {
532499
}
533500
};
534501

535-
/// TODO (rokhinip): The layout of the ActiveActorStatus seems to be broken in
536-
/// arm64_32 with priority escalation support, disable this for now.
537-
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION && SWIFT_POINTER_IS_4_BYTES
538-
#define SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION 0
539-
#endif
540-
541502
/// Similar to the ActiveTaskStatus, this denotes the ActiveActorState for
542503
/// tracking the atomic state of the actor
543504
///
@@ -904,24 +865,15 @@ static_assert(sizeof(ActiveActorStatus) == ACTIVE_ACTOR_STATUS_SIZE,
904865
/// We must either release the actor or create a new processing job
905866
/// for it to maintain the balance.
906867
///
907-
/// The current behaviour of actors is such that we only use the inline
908-
/// processing job to schedule the actor and not OOL jobs. As a result, the
909-
/// subset of rules that currently apply are (1), (3), (5), (6).
868+
/// The current behaviour of actors is such that we only have a single
869+
/// processing job for an actor at a given time. Stealers jobs support does not
870+
/// exist yet. As a result, the subset of rules that currently apply
871+
/// are (1), (3), (5), (6).
910872
class DefaultActorImpl : public HeapObject {
911-
friend class ProcessInlineJob;
912-
union {
913-
// When the ProcessInlineJob storage is initialized, its metadata pointer
914-
// will point to Job's metadata. When it isn't, the metadata pointer is
915-
// NULL. Use HeapObject to initialize the metadata pointer to NULL and allow
916-
// it to be checked without fully initializing the ProcessInlineJob.
917-
HeapObject JobStorageHeapObject{nullptr};
918-
919-
ProcessInlineJob JobStorage;
920-
};
921-
// This field needs to be aligned to ACTIVE_ACTOR_STATUS but we need to
922-
// hide this from the compiler cause otherwise it adds a bunch of extra
923-
// padding to the structure. We will enforce this via static asserts.
924-
char StatusStorage[sizeof(ActiveActorStatus)];
873+
// Note: There is some padding that is added here by the compiler in order to
874+
// enforce alignment. This is space that is available for us to use in
875+
// the future
876+
alignas(sizeof(ActiveActorStatus)) char StatusStorage[sizeof(ActiveActorStatus)];
925877

926878
public:
927879
/// Properly construct an actor, except for the heap header.
@@ -933,7 +885,6 @@ class DefaultActorImpl : public HeapObject {
933885
_status().store(status, std::memory_order_relaxed);
934886

935887
SWIFT_TASK_DEBUG_LOG("Creating default actor %p", this);
936-
JobStorageHeapObject.metadata = nullptr;
937888
concurrency::trace::actor_create(this);
938889
}
939890

@@ -987,26 +938,20 @@ class DefaultActorImpl : public HeapObject {
987938

988939
void deallocateUnconditional();
989940

990-
/// Schedule an inline processing job. This can generally only be
941+
/// Schedule a processing job. This can generally only be
991942
/// done if we know nobody else is trying to do it at the same time,
992943
/// e.g. if this thread just sucessfully transitioned the actor from
993944
/// Idle to Scheduled.
994-
void scheduleActorProcessJob(JobPriority priority,
995-
bool hasActiveInlineJob);
996-
997-
static DefaultActorImpl *fromInlineJob(Job *job) {
998-
assert(isa<ProcessInlineJob>(job));
999-
#pragma clang diagnostic push
1000-
#pragma clang diagnostic ignored "-Winvalid-offsetof"
1001-
return reinterpret_cast<DefaultActorImpl*>(
1002-
reinterpret_cast<char*>(job) - offsetof(DefaultActorImpl, JobStorage));
1003-
#pragma clang diagnostic pop
1004-
}
945+
void scheduleActorProcessJob(JobPriority priority);
1005946
};
1006947

1007948
} /// end anonymous namespace
1008949

1009-
static_assert(sizeof(DefaultActorImpl) <= sizeof(DefaultActor) &&
950+
// We can't use sizeof(DefaultActor) since the alignment requirement on the
951+
// default actor means that we have some padding added when calculating
952+
// sizeof(DefaultActor). However that padding isn't available for us to use
953+
// in DefaultActorImpl.
954+
static_assert(sizeof(DefaultActorImpl) <= ((sizeof(void *) * NumWords_DefaultActor) + sizeof(HeapObject)) &&
1010955
alignof(DefaultActorImpl) <= alignof(DefaultActor),
1011956
"DefaultActorImpl doesn't fit in DefaultActor");
1012957
static_assert(DefaultActorImpl::offsetOfActiveActorStatus() % ACTIVE_ACTOR_STATUS_SIZE == 0,
@@ -1206,32 +1151,19 @@ dispatch_lock_t *DefaultActorImpl::drainLockAddr() {
12061151
void DefaultActorImpl::deallocateUnconditional() {
12071152
concurrency::trace::actor_deallocate(this);
12081153

1209-
if (JobStorageHeapObject.metadata != nullptr)
1210-
JobStorage.~ProcessInlineJob();
12111154
auto metadata = cast<ClassMetadata>(this->metadata);
12121155
swift_deallocClassInstance(this, metadata->getInstanceSize(),
12131156
metadata->getInstanceAlignMask());
12141157
}
12151158

1216-
void DefaultActorImpl::scheduleActorProcessJob(JobPriority priority, bool useInlineJob) {
1217-
Job *job;
1218-
if (useInlineJob) {
1219-
if (JobStorageHeapObject.metadata != nullptr)
1220-
JobStorage.~ProcessInlineJob();
1221-
job = ::new (&JobStorage) ProcessInlineJob(priority);
1222-
} else {
1223-
assert(false && "Should not be here - we don't have support for any OOL actor process jobs yet");
1224-
// TODO (rokhinip): Don't we need to take a +1 per ref count rules specified?
1225-
swift_retain(this);
1226-
job = new ProcessOutOfLineJob(this, priority);
1227-
}
1159+
void DefaultActorImpl::scheduleActorProcessJob(JobPriority priority) {
1160+
Job *job = new ProcessOutOfLineJob(this, priority);
12281161
SWIFT_TASK_DEBUG_LOG(
12291162
"Scheduling processing job %p for actor %p at priority %#zx", job, this,
12301163
priority);
12311164
swift_task_enqueueGlobal(job);
12321165
}
12331166

1234-
12351167
bool DefaultActorImpl::tryLock(bool asDrainer) {
12361168
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
12371169
SWIFT_TASK_DEBUG_LOG("Thread %#x attempting to jump onto %p, as drainer = %d", dispatch_lock_value_for_self(), this, asDrainer);
@@ -1327,7 +1259,7 @@ void DefaultActorImpl::enqueue(Job *job, JobPriority priority) {
13271259
if (!oldState.isScheduled() && newState.isScheduled()) {
13281260
// We took responsibility to schedule the actor for the first time. See
13291261
// also ownership rule (1)
1330-
return scheduleActorProcessJob(newState.getMaxPriority(), true);
1262+
return scheduleActorProcessJob(newState.getMaxPriority());
13311263
}
13321264

13331265
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
@@ -1413,7 +1345,7 @@ bool DefaultActorImpl::unlock(bool forceUnlock)
14131345
if (newState.isScheduled()) {
14141346
// See ownership rule (6) in DefaultActorImpl
14151347
assert(newState.getFirstJob());
1416-
scheduleActorProcessJob(newState.getMaxPriority(), true);
1348+
scheduleActorProcessJob(newState.getMaxPriority());
14171349
} else {
14181350
// See ownership rule (5) in DefaultActorImpl
14191351
SWIFT_TASK_DEBUG_LOG("Actor %p is idle now", this);
@@ -1558,7 +1490,7 @@ static void defaultActorDrain(DefaultActorImpl *actor) {
15581490
// Leave the tracking info.
15591491
trackingInfo.leave();
15601492

1561-
// Balances with the retain taken in Process{Inline,OutOfLine}Job::process
1493+
// Balances with the retain taken in ProcessOutOfLineJob::process
15621494
swift_release(actor);
15631495
}
15641496

@@ -1590,14 +1522,6 @@ static void swift_job_runImpl(Job *job, ExecutorRef executor) {
15901522
}
15911523
}
15921524

1593-
SWIFT_CC(swiftasync)
1594-
void ProcessInlineJob::process(Job *job) {
1595-
DefaultActorImpl *actor = DefaultActorImpl::fromInlineJob(job);
1596-
1597-
swift_retain(actor);
1598-
return defaultActorDrain(actor); // 'return' forces tail call
1599-
}
1600-
16011525
// Currently unused
16021526
SWIFT_CC(swiftasync)
16031527
void ProcessOutOfLineJob::process(Job *job) {
@@ -1606,6 +1530,7 @@ void ProcessOutOfLineJob::process(Job *job) {
16061530

16071531
delete self;
16081532

1533+
// Balances with the swift_release in defaultActorDrain()
16091534
swift_retain(actor);
16101535
return defaultActorDrain(actor); // 'return' forces tail call
16111536
}

0 commit comments

Comments
 (0)