Priority inversion avoidance in Actor Runtime #41362
Conversation
|
@swift-ci please test |
|
|
||
| bool wasInlineJob() const { | ||
| return Kind == Inline; | ||
| } | ||
|
|
||
| static RunningJobInfo forOther(JobPriority priority) { |
There was a problem hiding this comment.
Since JobPriority of the processing job does not necessarily correlate to the priority of the actor we're processing, or the priority of the thread running that job (as a result of priority escalation that can happen asynchronously or as a result of self-overrides), I decided to remove it from this structured, since we can't make scheduling decisions based on this information.
|
Build failed |
|
@swift-ci please test |
|
Build failed |
| job = new (&JobStorage) ProcessInlineJob(priority); | ||
| } else { | ||
| assert(false && "Should not be here - we don't have support for any OOL actor process jobs yet"); | ||
| // TODO (rokhinip): Don't we need to take a +1 per ref count rules specified? |
There was a problem hiding this comment.
Sounds right. A comment pointing to the corresponding release would be handy.
| currentActor->giveUpThread(runner); | ||
| } | ||
|
|
||
| // We are eliminating a processing job for the original actor, release it. | ||
| swift_release(actor); |
There was a problem hiding this comment.
I'm actually about 80% certain that this is wrong to be here since we're only scheduling with inline jobs and we don't ever take extra ref counts on the actor since we don't create extra processing jobs.
Edit: Turns out we take a retain in ProcessInlineJob::process and ProcessOutOfLineJob::process so we need this release. But why do we need to take the retain there @rjmccall? Per your ownership rules, if anything it seems like we should be taking the retain at the time of scheduling the extra processing jobs?
rokhinip
force-pushed
the
rokhinip/86100521-actor-runtime-escalation
branch
from
84a8e5f to
72d15f6
Compare
|
@swift-ci please test |
rokhinip
force-pushed
the
rokhinip/86100521-actor-runtime-escalation
branch
from
72d15f6 to
3865ae6
Compare
|
@swift-ci please test |
| } | ||
|
|
||
| // Only for static assert use below, not for actual use otherwise | ||
| static constexpr size_t offsetOfActiveActorStatus() { |
There was a problem hiding this comment.
Only way I could think of doing this since the class members are private and so the static_assert() was unhappy.
state of the actor similar to the ActiveTaskStatus. Refactor the default actor runtime implementation to set us up for priority escalation support Radar-Id: rdar://problem/86100521
ActiveActorStatus Radar-Id: rdar://problem/86100521
Radar-Id: rdar://problem/86100521
DefaultActor Radar-Id: rdar://problem/86100521
rokhinip
force-pushed
the
rokhinip/86100521-actor-runtime-escalation
branch
from
3865ae6 to
a3e3c3f
Compare
|
@swift-ci please test |
This PR comes in 3 parts:
Rdar: rdar://86100521