Metadata and runtime support for checking isolated conformances at runtime #79785
Conversation
DougGregor
requested review from
rjmccall,
ktoso,
mikeash and
al45tair
as code owners
|
@swift-ci please test |
| @usableFromInline | ||
| internal static func _taskIsCurrentGlobalActor() -> Bool { | ||
| let executor = unsafe sharedUnownedExecutor | ||
| return unsafe _taskIsCurrentExecutor(executor: executor.executor, flags: 0) |
There was a problem hiding this comment.
Ok when the new isIsolatingCurrentContext I'm working on lands this will actually work then. Should be fine without changes I think (tho today it'll crash on not-isolated-to-the-expected-thing, which is expected). When run against an old runtime this must expect a potential of a crash basically I think, but in new runtimes it'll be good.
// ongoing discussions about how to roll out that new check with backwards compat etc.
There was a problem hiding this comment.
None of the code that would call this entry point back-deploys, so I think it's safe from crash-if-not-expected. If we did try to back-deploy, yeah, it would be a problem.
There was a problem hiding this comment.
There's more to that though; Say there's someone's custom executor impl; they did implement checkIsolated, they did not implement the new is... API. We check _taskIsCurrentExecutor against it; we have no choice (well we can debate what we should do), but to call the only "check" implementation we have -- the crashing one that the developer implemented. So regardless of backdeployment or not, these may crash if people only implemented the check... APIs.
If that happens, we should be telling people to instead implement the new in 6.2 isIsolatingCurrentContext and it'll stop crashing, but it's something to be aware of.
There's a big flowchart of what can get called when in swiftlang/swift-evolution#2716 -- we can come up with some other more drastic policy that we'd ignore implemented check methods but that's pretty dramatic... Anyway, something to be aware of as we discuss semantics of swiftlang/swift-evolution#2716
There was a problem hiding this comment.
Main executor has a bunch of special case sugar so it'll be fine; but this may come up for global actors on custom executors... there's probably pretty few of them so we may choose to not worry about this too much and just "if it blows up, update your impl" which is probably good enough, just highlighting that it's an issue that'll happen eventually
| @usableFromInline | ||
| internal static func _taskIsCurrentGlobalActor() -> Bool { | ||
| let executor = unsafe sharedUnownedExecutor | ||
| return unsafe _taskIsCurrentExecutor(executor: executor.executor, flags: 0) |
There was a problem hiding this comment.
Main executor has a bunch of special case sugar so it'll be fine; but this may come up for global actors on custom executors... there's probably pretty few of them so we may choose to not worry about this too much and just "if it blows up, update your impl" which is probably good enough, just highlighting that it's an issue that'll happen eventually
| // The concurrency library provides a function to check whether we | ||
| // are executing on the given global actor. | ||
| auto isCurrentGlobalActor = SWIFT_LAZY_CONSTANT(reinterpret_cast<bool (*)(const Metadata *, const WitnessTable *)>( | ||
| dlsym(RTLD_DEFAULT, "swift_task_isCurrentGlobalActor"))); |
There was a problem hiding this comment.
Could we avoid this by having the concurrency library register a function with libswiftCore? dlsym is problematic because it won't work on Linux when we're statically linked (and it doesn't exist on Windows either).
There was a problem hiding this comment.
We already do this sort of thing for Concurrency types with standard mangling. Look at the uses of ConcurrencyStandardTypeDescriptors. We could extend that facility to a more general "register stuff from the Concurrency runtime" thing, or add a second one.
There was a problem hiding this comment.
That's a great idea. This is runtime SPI, so I'll generalize it rather than adding a second one. Fewer __attribute__((constructor))s is a good thing
DougGregor
force-pushed
the
isolated-conformances-metadata-runtime
branch
from
1a7647a to
3e5be5c
Compare
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@swift-ci please test Windows |
|
@swift-ci please test macOS |
|
First time I broke wasm, yay? |
…ntime Extend the metadata representation of protocol conformance descriptors to include information about the global actor to which the conformance is isolated (when there is one), as well as the conformance of that type to the GlobalActor protocol. Emit this metadata whenever a conformance is isolated. When performing a conforms-to-protocol check at runtime, check whether the conformance that was found is isolated. If so, extract the serial executor for the global actor and check whether we are running on that executor. If not, the conformance fails.
…ents When establishing whether a given conformance is isolated, look through the witness tables used to satisfy conditional requirements as well. This is because an otherwise-nonisolated conditional conformance can become isolated if one of its associated conformance requirements is satisfied by an isolated conformance. While here, make sure this code works with variadic generics, too.
Following the approach taken with the concurrency-specific type descriptors, register a hook function for the "is current global actor" check used for isolated conformances.
In the prior implementation of runtime resolution of isolated conformances, the runtime had to look in both the protocol conformance descriptor and in all conditional conformance requirements (recursively) to find any isolated conformances. If it found one, it had to demangle the global actor type to metadata. Since swift_conformsToProtocol is a hot path through the runtime, we can't afford this non-constant-time work in the common case. Instead, cache the resolved global actor and witness table as part of the conformance cache, so that we have access to this information every time we look up a witness table for a conformance. Propagate this up through various callers (e.g., generic requirement checking) to the point where we either stash it in the cache or check it at runtime. This gets us down to a very quick check (basically, NULL-or-not) for nonisolated conformances, and just one check for isolated conformances.
Replace the pair of global actor type/conformance we are passing around with a general "conformance execution context" that could grow new functionality over time. Add three external symbols to the runtime: * swift_conformsToProtocolWithExecutionContext: a conforms-to-protocol check that also captures the execution context that should be checked before using the conformance for anything. The only execution context right now is for an isolated conformance. * swift_isInConformanceExecutionContext: checks whether the function is being executed in the given execution context, i.e., running on the executor for the given global actor. * swift_ConformanceExecutionContextSize: the size of the conformance execution context. Client code outside of the Swift runtime can allocate a pointer-aligned region of memory of this size to use with the runtime functions above.
…tor)) We don't have a great way to ensure that the current-global-actor hook will get installed by the concurrency library with WebAssembly, so temporarily work around the issue by relying on the fact that we also aren't doing actual concurrency with WebAssembly.
DougGregor
force-pushed
the
isolated-conformances-metadata-runtime
branch
from
5513db2 to
904d0cf
Compare
|
Based on an idea of @mikeash, I reworked the runtime portion of this to not require walking conditional requirements at lookup time. Instead, we store the isolated-conformance information within the conformance cache itself. The isolation of a conformance is determined at the point when we populate the cache (when we are already checking conditional requirements anyway), and is quick to check when the witness table was already cached. We cache the global actor type + its conformance to GlobalActor, so we also don't require demangle-to-type-metadata after the initiate entry is cached. The only memory over While here, tidy up the separation between "checks conformance but not the execution context" and "check the execution context" in a manner that can help us with other execution-time checks in the future. |
|
@swift-ci please test |
…constructor))" This reverts commit 8ef9f7f.
WebAssembly doesn't currently have a good way to install the "isCurrentGlobalActor" hook on which this checking depends. Disable the test for the moment.
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci please benchmark |
|
@swift-ci please test |
Extend the metadata representation of protocol conformance descriptors to include information about the global actor to which the conformance is isolated (when there is one), as well as the conformance of that type to the GlobalActor protocol. Emit this metadata whenever a conformance is isolated.
When performing a conforms-to-protocol check at runtime, check whether the conformance that was found is isolated. If so, extract the serial executor for the global actor and check whether we are running on that executor. If not, the conformance fails.
In its current state, this implementation is missing some pieces:
dlsymis ugly but probably unavoidable, since we need to downcall into the concurrency library. We'll need a separate implementation for Windows.MainActor-conforming generic parameter or associated type. The language doesn't handle this right now either, but the runtime should be more general.