2.x design decisions

[stevegury]

This is a first draft of the design document for the 2.x branch.
We are interested in collecting any feedback from the community (implementers but also pure users).

This present document is the collaborative work of @abersnaze, @benjchristensen, @stealthcode, and @stevegury. But we encourage anyone to propose improvements/clarifications via pull-request.

[akarnokd]

Extending both will prevent direct use of any instance of them directly.

Besides, the signature doesn't compile because it requires

void subscribe(io.reactivestreams.Subscriber)
void subscribe(java.util.concurrent.Flow.Subscriber)

to be implemented separately.

[stealthcode]

Right now we are not targeting Flow.Subscriber since this planned for Java 1.9. Our current source code compatibility is 1.6. I believe the rationale was to show that we intend that this would one day extend and interoperate with Flow.

This line should just be changed to ...

interface Subscriber<T> extends io.reactivestreams.Subscriber<T> {

[akarnokd]

an io.reactivex.Subscriber will just name-conflict with io.reactivestreams.Subscriber without any benefit.

[stealthcode]

So your proposal is to just remove this interface?

[akarnokd]

Yes.

[benjchristensen]

Good point if we don't need a concrete implementation (which we don't).

[akarnokd]

General observations

Class structure

I didn't see any mention of the architecture of the base objects and how one adds operators to it. As I suggested in my design retrospect post, we can save a lot of allocations by extending the base class instead of using create/lift internally all the time.

Error handling

In addition, I don't see any mention of requiring operators to call RxJavaPlugins.onError with exceptions that can't be delivered because of the RS contract. For example, if 2 sources on a regular flatMap signal an error, only the first can be delivered reliably and the other has nowhere to go.

Consequently, since RS subscribers shoudn't really throw in their onXXX methods but end-subscribers tend to (as evidenced by many issue list reports), we'll need a safeSubscribe() method as well. As a result, we can no longer throw OnErrorNotImplementedException with subscribe() and subscribe(Consumer) because a) we don't know where it will bubble up and b) if it reaches the scheduled action root, it still will end up in RxJavaPlugins.onError.

Default Subscriber implementations

Since cancelling and disposing is now potentially racy, we can't really expect end-subscribers to implement logic for them in their onSubscribe() calls. We need standard subscriber classes that do any internal exposure (access to Subscription.request conveniently) or external exposure (a cancellable AsyncObserver and AsyncSubscriber)

Operator fusion

Macro-fusion relies on the property that operator classes can be identified in some way. For example, just() optimizations work in 1.x because it was implemented as a class extending Observable instead of hidden behind create(). However, as long as the Observable returned by create() can be unwrapped to look at the real OnSubscribe class, the fusion can still work, albeit with more (unnecessary) overhead. Using lambdas or anonymous inner classes defeats it.

Micro-fusion requires a library-internal protocol to work amongst operators. One day, it would be great to have standard fusion protocol library that all RS libraries can implement and do cross-library fusion with it.

Changes to 2.x master

I see the following changes required in current 2.x master:

1. rename Observable to Flowable, including operators prefixed
2. rename NbpObservable to Observable, including operators prefixed
3. change Observer to interface, fix the depending sites
4. make PublishSubject and BehaviorSubject extend Observable instead of Flowable
5. remove operators from Flowable that would signal onError if the downstream can't keep up
6. rewrite lifted operators to class-extension form
7. add missing operators and overloads from 1.x,
8. add missing unit tests from 1.x
9. remove unnecessary operators or overloads
10. add JavaDoc
11. define interfaces and protocol for fusion, start applying operator-fusion to Flowable
12. define interfaces and protocol for non-backpressured fusion, start applying operator-fusion to Observable

[JakeWharton]

What advantage is there for producing two artifacts? If you're not using JDK 8 APIs why not just make the JDK 6 artifact the canonical one.

Having two makes me very nervous about binary compatibility with the two. Java projects built against the JDK 6 impl which aim to support Android should be binary compatible with JVM users on Java 8+. Either a mechanism should be put in place to verify binary compatibility (e.g., animal sniffer) or it might just be easier to have only a single output artifact.

[ZacSweers]

Also, how would you include sources for the jdk6 distribution since retrolambda only operates on the bytecode? It would make the IDE have to decompile the classes to represent them rather than being able to just click in. Would this not make debugging unnecessarily more tedious and also sacrifice javadoc/comment access?

[stealthcode]

I think this is a great point. We of course do not want to cause unnecessary friction in any subset of user base. The bytecode compatibility can easily be tested as per the proposals in #3764 (see comment).

[benjchristensen]

I prefer having a single artifact.

[stealthcode]

@akarnokd

we can save a lot of allocations by extending the base class instead of using create/lift internally all the time.

I don't see any mention of requiring operators to call RxJavaPlugins.onError with exceptions that can't be delivered because of the RS contract.

disposing is now potentially racy

Macro-fusion relies on the property that operator classes can be identified in some way.

These sound like additive changes to the existing design revision. Would it be okay if you make a revision to the DESIGN.md document and open a PR with proposals to address these items?

I see the following changes required in current 2.x master:

Can you hold off on opening PRs until the DESIGN.md changes have been merged into 2.x? This should serve as a sign that the decisions have been reviewed and commitment has been reached by the RxJava committers. You are of course welcome to play around with code for your own understanding and demonstration purposes.

[akarnokd]

Would it be okay if you make a revision to the DESIGN.md document and open a PR with proposals to address these items?

Sure.

Can you hold off on opening PRs until the DESIGN.md changes have been merged into 2.x?

Yep.

[benjchristensen]

I suggest merging this document so we can start modifying portions of it via PRs. There appear to be no general disputes, but valid changes to individual items that should be done.

[benjchristensen]

It's been a week. Can we merge?

[akarnokd]

Fine, 👍, let's use it as a starting point.

[benjchristensen]

Merged. Let's start making changes via small pull requests on the various details that were discussed above.

[ZacSweers]

It's unfortunately been a month and nothing new has been contributed to the 2.x branch. Netflix folks, @akarnokd, and others: would it be useful create a slack group for reactivex/RxJava to facilitate discussions amongst each other and the community rather than strictly GitHub correspondence? It seems like the latter is adding unnecessary friction to discussions since it doesn't scale well to many people discussing at once, let alone its inherent slowness conpared to chat. Could help improve organization and planning amongst contributors as well. Would be happy to help organize this, and I know many other developers have wished there was a resource like it having similar ones for other platforms and frameworks.

[akarnokd]

My latest refactor PR is still not merged so I can't really start pushing other things. If you want, you can set up a chat somewhere, but note I'm at GMT+2.