Skip to content
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

Introduce non-clone streams #508

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

antiguru
Copy link
Member

@antiguru antiguru commented Mar 3, 2023

At the moment, any data sent through a Timely stream needs to be Clone because Tee needs to be able to clone data to send it to multiple recipients. This change tries to change this, although it comes with far-reaching and breaking changes.

I don't intend to merge this yet, but it seems like a solid basis for further experiments.

Specifically, it introduces new types and changes behavior:

  • Operators by default produce an OwnedStream that can only be connected to one downstream operator. Because of this, there's no Tee involved that would require data to be Clone.
  • To force a Tee, OwnedStream has a tee function that returns a stream that can be shared between downstream operators. This function requires the data to be Clone and just like all other operators consume the stream.
  • Operators are implemented for a StreamLike type to be generic over OwnedStream and StreamCore. This requires a few more type parameters here and there.
  • It removes the Clone requirement from Container. This adds a bound +Data requirement to all operators that want to take ownership of their container input (which is all operators.)
  • The names are .. not great. Open to suggestions.

The effect of this change is that for op --> op structures, we only have a vcall where we currently have a vector dereference plus vcall. This can be better in some situations, but I didn't measure it. The downside is that requesting a tee adds the vector dereference plus vcall after the first vcall, so it's strictly worse. This should be amortized by how infrequently it's used, but who knows.

In theory, we don't need the vcall at all because the type of the downstream operator is known to Rust and hence its pusher. However, I currently can't see how to wire it up, and I believe it's ugly because the information needs to flow backwards from the receiving operator to the source.

@antiguru antiguru force-pushed the owned_stream branch 2 times, most recently from 465f715 to ad6150a Compare March 5, 2023 02:59
@antiguru antiguru changed the title Work on owned StreamCores instead of references Introduce non-clone streams Mar 6, 2023
@antiguru antiguru marked this pull request as ready for review March 6, 2023 02:30
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
Signed-off-by: Moritz Hoffmann <[email protected]>
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.

1 participant