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

AsyncChannel naming is provisional and needs review #47

Closed
phausler opened this issue Feb 17, 2022 · 12 comments
Closed

AsyncChannel naming is provisional and needs review #47

phausler opened this issue Feb 17, 2022 · 12 comments
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed review Naming or behavior is provisional and needs further review. v1.0 Work leading up first API stable version

Comments

@phausler
Copy link
Member

phausler commented Feb 17, 2022

The naming of AsyncChannel was introduced as a placeholder name. It deserves more consideration to existing paradigms, the current name is inspired from Channel. This type needs a guide and some research/discussion on what a good name for this type should be.

@phausler phausler added documentation Improvements or additions to documentation help wanted Extra attention is needed review Naming or behavior is provisional and needs further review. labels Feb 17, 2022
@ktoso
Copy link
Member

ktoso commented Feb 18, 2022

From a quick skim of the implementation and tests it seems Channel might be a good name for it hm hm 👍
Need to think if we need to qualify it with some prefix or not -- when does the send return?

@phausler
Copy link
Member Author

So the send returns in this case as soon as someone receives the value in an iterator. So it has the "async on both sides" behavior to ensure back pressure is respected.

@phausler phausler changed the title AsyncSubject naming is provisional and needs review AsyncChannel naming is provisional and needs review Mar 1, 2022
@jmjauer
Copy link

jmjauer commented Mar 26, 2022

I think the name AsyncChannel is well chosen - it is short and describes exactly the semantics.

@andersio
Copy link

andersio commented Mar 27, 2022

Kotlin Coroutines names this RendezvousChannel. Though the "rendezvous" prefix is most likely a result of them also offering buffering variants [1] under the same Channel umbrella.

[1] sender suspends on full; receiver suspends on empty.

@twittemb
Copy link
Contributor

twittemb commented Mar 31, 2022

Is there an official definition to what a Channel should be? especially regarding the synchronisation part between sender and receiver.

If the link @phausler has provided is considered an "official" definition, then I'm not sure we should keep the Async prefix.

Channel alone would simply mean that we have implemented the official Channel paradigm with async/await as an implementation detail. Moreover AsyncChannel could be misleading by thinking Async means that sending a value is not bound to its consumption.

On the other hand, if the accepted definition of Channel does not imply the synchronisation between sender and receiver, then I guess Channel is a family of different implementations depending on the buffering policy (like Kotlin). If we want to go down that road then something like RendezVousChannel or similar could be interesting in comparison with a future BufferedChannel (which would be Channel + AsyncBufferSequence)

@parkera parkera added the v1.0 Work leading up first API stable version label May 4, 2022
@paulofaria
Copy link

I think Channel and BufferedChannel works pretty well. I'm not sure the Rendezvous prefix buys us much. Also, since we're talking about BufferedChannel is there any work being done on that front? If not, I'm happy to contribute with an implementation/proposal.

@paulofaria
Copy link

Actually, this is (probably?) not possible to implement now, but it would be awesome if we could add deadlines to Channels like we did on Venice (backed by libdill). Like the test excerpt below:

    func testDoubleSendTimeout() throws {
        let channel = try Channel<Int>()

        let coroutine1 = try Coroutine {
            XCTAssertThrowsError(
                try channel.send(111, deadline: 50.milliseconds.fromNow()),
                error: VeniceError.deadlineReached
            )
        }

        let coroutine2 = try Coroutine {
            XCTAssertThrowsError(
                try channel.send(222, deadline: 50.milliseconds.fromNow()),
                error: VeniceError.deadlineReached
            )
        }

        try Coroutine.wakeUp(100.milliseconds.fromNow())

        let coroutine3 = try Coroutine {
            try channel.send(333, deadline: .never)
        }

        XCTAssertEqual(try channel.receive(deadline: .never), 333)

        coroutine1.cancel()
        coroutine2.cancel()
        coroutine3.cancel()
    }

If not possible right now, what would we need in place to make it happen?

@paulofaria
Copy link

I moved the discussion above to its own issue.

@eaigner
Copy link

eaigner commented Dec 10, 2022

I would just model it 1:1 like channels in Go, so as for the name AsyncChannel would be correct.

Why reinvent the wheel?

On a different note, close seems like a more reasonable name for the finish method.

@FranzBusch
Copy link
Member

IMO the current semantics fit very well with what we want to achieve with this type. The intention is to have a multi-producer multi-consumer root asynchronous sequence that allows lock-step programming between the involved parties. This is exactly what this type offers.

@FranzBusch
Copy link
Member

@phausler I think we can close this issue now?

@phausler
Copy link
Member Author

Yea, the name feels pretty solid and has been reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Extra attention is needed review Naming or behavior is provisional and needs further review. v1.0 Work leading up first API stable version
Projects
None yet
Development

No branches or pull requests

9 participants