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

Consider keeping BroadcastChannel in a deprecated state for longer #2680

Closed
Artraxon opened this issue Apr 27, 2021 · 8 comments
Closed

Consider keeping BroadcastChannel in a deprecated state for longer #2680

Artraxon opened this issue Apr 27, 2021 · 8 comments

Comments

@Artraxon
Copy link

I have just read the news that Broadcast Channel is going to be completely removed with next release.
Why? I have build my unexBot and tihibot using the Channel API because it was simpler and less prone the change. The Flow API was and is still constantly changing and I simply do not need the added functionality.

I understand that this is an experimental API, but If you leave an "experimental" API out there for 4 years, It should be expected that people are going to use it. And two major versions (effectively 2 years), of which one is also unmature, so 1 year in the end, for swapping out ALL Channel usage is completely reckless. What is really the point in removing it? Mark it as deprecated and refer to the alternatives and then leave it at that for some years, it does not do any harm, only removing it does.

The people that are building many of the open-source libraries for kotlin are not working full-time on there open-source projects. I'm not. I already started refactoring these bots to move the Framework that I've written into a separate library, but now I don't see the point because it's going to be broken anyway in a year and I do not have the time to fix it.

@elizarov
Copy link
Contributor

Thanks. We'll consider leaving it out there for longer. What is your suggested timeline? Is having it in a deprecated state for, say, a year enough?

@elizarov elizarov changed the title Why deprecate BroadcastChannel? Consider keeping BroadcastChannel in a deprecated state for longer Apr 27, 2021
@Artraxon
Copy link
Author

Speaking for me personally: I don't know, that massively depends both on how my life and how the coroutines library develop.
Of course, more time is always better.

But still, what is the reasoning behind this? Why not leave a low-level API in place? Especially considering that what is intended to replace BroadcastChannels is way less mature then those.

Everytime one of these things get removed it breaks projects and libraries.
Also, if this gets removed after many years of its existence, how can I be sure that sharedFlow is not going to be altered in a breaking way soon after again?

What should my strategy be on deciding what to use from the Kotlin STDlibs and what not when I want my programs to be running in five or ten years without having to refactor it with every major release?

@Artraxon Artraxon reopened this Apr 27, 2021
@elizarov
Copy link
Contributor

There is always a tradeoff. The more code a library has, the harder it is to maintain and to expand. We need to run all the tests for it, we need to maintain the code to make sure it works with all the latest versions of Kotlin, etc.

BroadcastChannel in particular, is extremely problematic because it extends a Channel interface, which is a crucial core interface of a coroutines library and we have future plans to evolve it to allow for a much more efficient/performant implementation of various channels. BroadcastChannels, when they are still in the library, pose a considerable hindrance to those future plans.

@pacher
Copy link

pacher commented Apr 28, 2021

I am personally not affected by the deprecation and would prefer faster pace of development of new features vs keeping deprecated.
However, just to be fair I want to point out that migration from channels to SharedFlow might be not trivial since they are semantically not the same thing.

The big elephant in the room of SharedFlow design is error handling. BroadcastChannel design and similar primitives in Rx propagate errors from upstream to downstream.

Channels can be closed and can fail. SharedFlow never completes.

Quoting @elizarov from here:

@pacher We are considering the option of providing a ready-to-use wrapper and operators to materialize/dematerialize completion and errors for sharing so that you don't have to write them yourself. The core design presented here is mostly aimed at sharing never-ending event flows.

So basically if you remove Broadcast Channels now, there will be nothing for sharing of limited-sizes flows. Also nothing for the case of replicating the flow and where one wants to propagate an error.

Everybody migrating to SharedFlow will have to at least implement materialize/dematerialize pair by him(her)self.
I remember I tried and there were couple of gotchas, but unfortunately I don't remember the details.

@Artraxon
Copy link
Author

BroadcastChannel in particular, is extremely problematic because it extends a Channel interface, which is a crucial core interface of a coroutines library and we have future plans to evolve it to allow for a much more efficient/performant implementation of various channels. BroadcastChannels, when they are still in the library, pose a considerable hindrance to those future plans.

Without knowing the exact details of the implementations I guess I have to acknowledge that has a good reason. What I am using Broadcast Channels for looks effectively like what SharedFlow is supposed to to, but I don't feel well with moving to something that has been out there for not even a year and therefore is prone to quick changes (which this discussion itself is a good example for).

Besides removing BroadcastChannels (and ConflatedBroadcastChannels), what other changes are planned for the Channels API?

@elizarov
Copy link
Contributor

We plan to reworks internal Channel APIs that determine their interaction with the select {} expression. Flow will not be affected since it doesn't work with select at all, but a BroadcastChannel will because it is a Channel. We don't really have a way of supporting an improved high-performance select infrastructure in the BroadcastChannel. As a compromise, we can keep implementations of BroadcastChannel in the library for longer, but throw an exception on any attempt to use them with select. That'll let some simpler code that uses them to avoid migration for longer while giving us the ability to roll out improved channel infrastructure.

@Artraxon
Copy link
Author

Thank you, that sounds very reasonable!

@qwwdfsad
Copy link
Member

It was quite a lot of time since hot flows were introduced and broadcast channels were marked as obsolete.
Since #3621 we found that the maintenance of broadcast channels is becoming more and more harder to the extent where the very design of broadcast channels prevents us from a more efficient solutions within channel's core.

We do plan to deprecate them for good starting from 1.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants