Skip to content

Remove Sendable constraint for AsyncAdjacentPairsSequence #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

FranzBusch
Copy link
Member

Motivation

We want to remove any Sendable constraints on iterators since they ought to not be passed between Tasks.

Modification

Explicitly mark the AsyncAdjacentPairsSequence to be not Sendable.

Result

Nobody can make this Sendable.

@FranzBusch FranzBusch requested a review from phausler October 12, 2022 09:22
# Motivation
We want to remove any `Sendable` constraints on iterators since they ought to not be passed between `Task`s.

# Modification
Explicitly mark the `AsyncAdjacentPairsSequence` to be not `Sendable`.

# Result
Nobody can make this `Sendable`.
@phausler
Copy link
Member

Assume Base.Iterator: Sendable

Sendable requires all usages across tasks to be well formed

usage of an iterator across tasks can result in two tasks deriving the same value (broadcast) or deriving subsequent values (multicast)

any conditional conformance of Sendable must preserve the usage form (broadcast or multicast) characteristic

given the same values upstream the adjacent pairs downstream by definition should be the same
given an upstream of 1, 2, 3, 4 results in (1, 2), (2, 3), (3, 4)

if the upstream is broadcast then the adjacent pairs would be the same

if the upstream is multicast and provides 1, 2, 3, 4 it can be split into two via one producing 1, 3 and another producing 3, 4 then the downstream would be (1, 3) and (3, 4)

does that latter violate the multicast characteristic?

@phausler
Copy link
Member

I was able to prototype a version of a broadcast algorithm that does not require the iterators to be sendable - so this change is fine!

@phausler phausler merged commit c21ee2d into apple:main Oct 14, 2022
@FranzBusch FranzBusch deleted the fb-adjacent-pairs branch October 26, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants