[SE-0526] Feedback and refinements for withTaskCancellationHandler, and CancellationError.Reason#3306
Conversation
|
Sharing some implementation notes which don't necessarily have to be part of the proposal but this seems like a good place to post it as from this we should form the parts that do end up in the proposal update. Reading deadlinesWe definitely want a way to read a deadline, it's a hard requirement, so let's see how we can achieve it. There's two approaches we can take; The first one Philippe argues is more useful, however it is more complex. It has to convert all instants into the ContinousClock.Instant -- it is "easier" for users, however slower to lookup and it may answer slightly "different" times because of conversions as it needs to convert other clocks deadlines into the ContinousClock. It is an open question how this approach would work for entirely custom clocks still a little bit... One option is to say they just say "now" and we leave it at that, but this oculd be VERY confusing I worry... First: Philippe has an approach where we have to iterate all deadlines and get all clocks to convert into an "best approximation" of ContiniousClock.Instant.
To implement this visitor, we cannot do it in pure Swift (to be efficient, no horrible super hacks):
The visitor would have to be a protocol, since we don't have generic function literals, so something like this: // NOT necessarily exposed as API
protocol DeadlineVisitor {
func visit<C: Clock>(clock: C, deadline: C.Instant) -> ()
}
From runtime this is called as // This "existential" box has to be hand-defined in the
// runtime because we don't have existentials like this in
// Swift. And we can take advantage of ownership a bit here,
// as if our existential actually were:
// \exists C: Clock . (Ref<C>, Ref<C.Instant>)
struct {
Metadata *C;
Clock_WitnessTable *C_conforms_to_Clock;
void *clock; // pointer to a borrowed C
void *deadline; // pointer to a borrowed C.Instant
// maybe tolerance too?
}So we implement this iteration for the purpose of implementing this "adjusted" API: extension Task where Success == Never, Failure == Never {
public static var currentEarliestContinuousDeadline: ContinuousClock.Instant? { get }
}The implementation of this would need to iterate all deadlines though... and then convert all instants into the ContinuousClock.Instant and call a new requirement on Clock: protocol Clock ... {
// New addition:
func earliestContinuousInstant(from instant: Instant) -> ContinousClock.Instant
}
extension Clock {
func earliestContinuousInstant(from instant: Instant) -> ContinousClock.Instant {
.now // the default... also for custom clocks
}
}This is kinda true for all clocks. Any deadline may trigger at the earliest "now" but it is also kinda potentially be misleading. This would be possible to implement. Second The second is simpler, but will require us to give clocks Identity. It's a miss that we didn't do this before and thanks to this we can then offer the The benefit of this approach is:
Lookups need to traverse records only until they hit the first record where the clock type and the clock identity match the lookup. This often can be a single quick lookup; it never needs to scan past a hit, like the "adjusting lookup" would need to do. This is way cheaper to implement, but we do need to solve clock identity. Thanks for collaborating on figuring this out @FranzBusch, @phausler and @rjmccall; |
|
@ktoso Interesting write-up. I think the second approach would be the sensible solution here. But do I understand you correctly that "give clocks Identity" would be something public-facing that each clock implementation can customize? |
e9b28a5 to
2111a73
Compare
|
I don't think we can really give clocks identity. Especially retroactively. |
Of course we can, as we did for executors. New protocol requirement with default impl, without the |
…ationHandler overload and add some notes about the CancellationError.Reason
…us clocks and deadlines
f53ec9d to
8252bde
Compare
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
…led and adjust some wording/formatting to be clearer
Co-authored-by: Franz Busch <privat@franz-busch.de>
|
Not sure how you applied the change @phausler but it didn't work; the code snippet is not present. Just the sentence "Furthermore, the cancellation reason can be retrieved from the current Task similar to how cancellation is checked." without showing the API is not sufficient really. Easiest would be to take phausler#1 |
|
Thanks that works too :) |
This adds in a bit more refinements for the CancellationError.Reason and explains the future directions and constraints around that along with an overload for withTaskCancellationHandler