[Pitch] Cheap Task identity for high-performance instrumentation

Evolution Pitches
21

Here's a first draft of the proposal if you want to have a look:

5 Likes
22

Thank you for writing this, one of the well written proposals which I have read.

I thought about the counter-space depletion, and I found it addressed in the proposal.

Counter-space exhaustion (2⁶⁴ identifier allocations within a single process) is not specified behaviour. The 64-bit space is large enough to make this unreachable in any realistic process lifetime — at one billion task allocations per second it would take roughly 584 years to wrap — so the proposal does not mandate a specific outcome.

584 years is long! :slight_smile:

2 Likes
23

Thanks @hassila, proposal looks good -- that's exactly what we need to do here. Agree on the lack of public init and not over-conforming it to things.

The guarantees we provide there are OK; It's good to not promise too much about the value but just focus on uniqueness, thanks.

I’ll flesh up my impl to match this and put it up yoday

2 Likes
24

Here's an initial implementation: Introduce Task.id for quickly identifying tasks by ktoso · Pull Request #89237 · swiftlang/swift · GitHub

The only change from the proposal I'd argue for would be to make it TaskID and Task.ID, because of consistency with Identifiable.ID - what do folks think about that?

We could debate the backdeployment some more; We're able to do the get task + get id from job in older runtimes but it'll cause ARC on the task handle; so the new _getCurrentTaskId is more efficient; we could only use the new way in newer runtimes but make the API available on as old runtimes as _getJobTaskId allows :thinking:

@hassila

1 Like
25

Note that availability checks have a nonzero overhead, so it’s worth evaluating if doing the check is actually better than unconditionally using the old ABI if the deployment target is below the threshold.

3 Likes
26

Yeah that is a good point; cheapest of course would be to not check at all and just keep this using the latest API and have the higher @availability on the new API entirely :thinking:

I'll do some more experimentation on this front in the coming days.

1 Like
27

Thanks for the implementation @ktoso - will have a look!

Personally I'd be more than happy to make sure it just uses the latest API and can't backport so far for any improvements in performance really, I'd prefer newer runtimes only but lowest possible overhead. (one can always use the current unsupported way to get the ID for older runtimes if needed - nicer to get it right long term)

Task.ID is fine by me, happy to amend!

3 Likes
28

Updated with new spelling and a link to implementation.

29

Great to see this progressing. I guess there's no harm having it without "unsafe" in its name.

In the PR I see this comment:

SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
uint64_t swift_task_getCurrentTaskId(void);

However if I try a very similar (and perhaps equivalent) fragment myself I do not see any ARC traffic surfacing:

@_silgen_name("swift_task_getJobTaskId")
private func _swift_task_getJobTaskId(_ job: UnsafeMutableRawPointer) -> UInt

@_silgen_name("swift_task_getCurrent")
private func _swift_task_getCurrent() -> UnsafeMutableRawPointer?

var currentID: UInt? {
    guard let task = _swift_task_getCurrent() else { return nil }
    return _swift_task_getJobTaskId(task)
}

Am I missing anything?

1 Like
30

Hah, that's sneaky actually... and we could do the same actually huh. Here's what's happening:

The reason this did incur ARC traffic in my branch is because in the stdlib those APIs are not returning unsafe pointers; they return Builtin.NativeObject, which is ARC managed.

Since you can't use builtin types outside the stdlib, the way you spelled the signature to get that object as a raw pointer basically side stepped this. To be honest that's actually fine, and we could just do another version of the API treating the task as a raw pointer like this, that's a good idea actually :slight_smile: