[Concurrency] Decode actor/task flags in signposts, make task_wait an interval. #41808
Conversation
… interval, signpost continuations. Decode all fields from the various flags values, pass each field as a separate argument to the various signposts. We were just passing the raw value of the flags and requiring the signpost client to decode them, which was ugly and required the client to know details they shouldn't need to know. Strip ptrauth bits from the task resume function when signposting, when ptrauth is supported. Add signpost events for continuation init/await/resume events. We also make task_wait into an interval, rather than a single event. The interval ends when the task resumes. As part of this change, we also skip emitting the interval when the wait completed immediately and the task didn't have to suspend. While we're in there, clean up a few SWIFT_TASK_DEBUG_LOG lines that emitted warnings when built with the logging enabled. rdar://88658803
| // the enum values, but this explicit conversion provides room for change. | ||
| uint8_t traceState = 255; | ||
| switch (getActorState()) { | ||
| case ActiveActorStatus::Idle: |
There was a problem hiding this comment.
Should these values be exported in a private header somewhere for Instruments to slurp up?
And maybe we have a version number as well?
There was a problem hiding this comment.
Or do we think we'd just change the signpost ID value if we had a breaking change?
There was a problem hiding this comment.
I think a header probably wouldn't make things any easier. @Harjas12 any thoughts on that?
We have several options for making changes. We can refrain from using traceState values that are no longer in use and add new ones, we can keep this parameter in some sort of degraded state and add a new one with new info, or we can change the signpost ID. Adding a version number doesn't give us anything particularly useful on top of that.
There was a problem hiding this comment.
So I think the best way to handle this is to add the new values by appending to the log string. Parsers should be smart enough that the old versions ignore anything new and new version know to ignore the old fields and look at the new fields. If we do have to "deprecate" a field, we would ideally keep it in a good enough state for 1 year before we completely removed it. Any changes that remove from the format would need a 1 year lead time.
|
@swift-ci please test |
|
@swift-ci please test linux platform |
|
@swift-ci please smoke test macos platform |
|
Nice, looking good :) I noticed the failure was a distributed test -- looking into it -- ok the failure was #41840 so that should be ok now (and it'd was only on 32bit watch anyway). |
|
@ktoso Thanks for checking into that. Sorry, I should have pinged you when I saw the failure. I get so very focused on getting to a mergable state sometimes.... |
Decode all fields from the various flags values, pass each field as a separate argument to the various signposts. We were just passing the raw value of the flags and requiring the signpost client to decode them, which was ugly and required the client to know details they shouldn't need to know.
Strip ptrauth bits from the task resume function when signposting, when ptrauth is supported.
Add signpost events for continuation init/await/resume events.
We also make task_wait into an interval, rather than a single event. The interval ends when the task resumes. As part of this change, we also skip emitting the interval when the wait completed immediately and the task didn't have to suspend.
While we're in there, clean up a few SWIFT_TASK_DEBUG_LOG lines that emitted warnings when built with the logging enabled.
rdar://88658803
rdar://88597345