Skip to content

Commit 2adeeb5

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 efbd7cd commit 2adeeb5

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
@@ -421,20 +421,6 @@ namespace {
421421

422422
class DefaultActorImpl;
423423

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

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

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

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

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

@@ -986,26 +937,20 @@ class DefaultActorImpl : public HeapObject {
986937

987938
void deallocateUnconditional();
988939

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

1006947
} /// end anonymous namespace
1007948

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

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

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

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

13321264
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
@@ -1412,7 +1344,7 @@ bool DefaultActorImpl::unlock(bool forceUnlock)
14121344
if (newState.isScheduled()) {
14131345
// See ownership rule (6) in DefaultActorImpl
14141346
assert(newState.getFirstJob());
1415-
scheduleActorProcessJob(newState.getMaxPriority(), true);
1347+
scheduleActorProcessJob(newState.getMaxPriority());
14161348
} else {
14171349
// See ownership rule (5) in DefaultActorImpl
14181350
SWIFT_TASK_DEBUG_LOG("Actor %p is idle now", this);
@@ -1557,7 +1489,7 @@ static void defaultActorDrain(DefaultActorImpl *actor) {
15571489
// Leave the tracking info.
15581490
trackingInfo.leave();
15591491

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

@@ -1589,14 +1521,6 @@ static void swift_job_runImpl(Job *job, ExecutorRef executor) {
15891521
}
15901522
}
15911523

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

16061530
delete self;
16071531

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

0 commit comments

Comments
 (0)