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

Support API compatibility with Java 6 in 2.x #3450

Closed
JakeWharton opened this issue Oct 16, 2015 · 20 comments
Closed

Support API compatibility with Java 6 in 2.x #3450

JakeWharton opened this issue Oct 16, 2015 · 20 comments
Milestone

Comments

@JakeWharton
Copy link
Contributor

(Continuing the discussion from Twitter)

There is currently very little use of Java 8 APIs which provides significant value to the 2.x codebase. Additionally, the use of RxJava on Android continues to grow, along with concern about future use of 2.x. I believe we should target Java 6 API compatibility for the 2.x codebase (just like 1.x currently does), but with the key difference that the Java 8 language level remain in use for source files.

That last distinction is very important as it allows Retrolambda to be used as a bytecode transformation tool to move lambdas, method references, and default/static methods on interfaces to be compatible with the Java 6 bytecode.

There's two APIs which are currently used in a high-leverage way:

  • ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean)
  • StampedLock

StampedLock is part of JSR-166 and is placed in the public domain. It has minimal dependency on other types (basically just a tiny subset of ThreadLocalRandom, also in the JSR) and easily embedded in the library.

As to setRemoveOnCancelPolicy, there's already a workaround in place for 1.x and the very same could be used for 2.x.

Aside from that there's groupings of Java 8 APIs in use which are easily eliminated:

  • Optional - Public APIs using it are limited and likely could just be dropped. Internal use can be replaced with something else (e.g., Box<T>).
  • CompletableFuture - Used only in factory methods, can be moved to sibling Java 8 mix-in module.
  • Stream - Used only in factory methods (see above) and internally as a provider of convenience operators around an iterator (find first, find last, etc.). These are easily replaced with static methods that operate on Iterable directly. Guava has Apache 2-licensed versions that can be copy/pasted.
  • java.util.function.* - Easily replaced with SAM interfaces alongside the other FunctionN classes. Since Java 8's type inference allows matching SAM signatures to be used interchangably this should have no effect on usage of public API.
  • Random default / static helper methods from JDK types (e.g., Long.compare) - Easily replaced with ones in the internal package.

The Animal Sniffer tool can be used to automate detection of Java 8 APIs in use, and enforce at compile-time that none are used. I am currently using this in my backport attempt to great effect.

With this change, support for things like j.u.c.Flow (which won't be in until Java 9 anyway) and perhaps value types will all be grouped into a hypothetical future 3.x.

@artem-zinnatullin
Copy link
Contributor

If JDK 8 API is not a requirement for RxJava v2, probably neither should be a requirement of Java 8 lang syntax.

Though we use Retrlolambda in production I can not say that it's a perfect tool, once per month or so we have problems with it (usually minors, but it's a fact).

Benefits of using Java 8 lang features seem low for a library. Without JDK 8 APIs it's just lambdas, default methods in interfaces and try-with-resources. A possible spectrum of problems with Retrolamda: some JDK releases break compilation, build tools integration between versions of Gradle and Gradle Plugins, third-party dependencies integration, etc. Possible profit — slightly more readable (for library developers) and compact code for RxJava and more reported bugs for Retrolabmda :)

For such Generics-yfied library as RxJava, type-inference with lambdas may play a bad joke and lead to releases with incorrect type bounds on generics in some operators and other APIs. Also, it will make reading of RxJava sources harder for the application developers, since RxJava is not the easiest code to understand, type inference and lambdas will hide probably useful info from the reader.

Yes, Java 8 will give nicer code and productivity of library developers will increase for some %, but time spent on dealing with bytecode modification problems may be bigger. We've survived with Java 6 syntax in RxJava 1 without problems, why v2 requires Java 8 lang syntax?

As I said in Twitter discussion — huge 👍 to compatibility with Java 6 in RxJava v2. But +- to Retrolambda and Java 8 syntax for a library, not arguing against it, but adding some thoughts.

@iNoles
Copy link

iNoles commented Oct 16, 2015

http://www.javaworld.com/article/2990440/mobile-java/oracle-considers-a-new-effort-to-develop-mobile-java-apps.html This is very interesting article because Oracle wants to do JDK9 port to Android, iOS and Windows Mobile.

@akarnokd akarnokd added this to the 2.0 milestone Oct 16, 2015
@dlew
Copy link

dlew commented Oct 16, 2015

Question (since the roadmap isn't clear): What is the plan for 1.x going forward? Is there dual support planned, or is the future going to consist mostly of 2.x+?

I'd prefer that 2.x support Android, but that becomes doubly important if 1.x support will cease once 2.x is released.

@akarnokd
Copy link
Member

I've finished the backport:

https://github.com/akarnokd/rxjava2-backport
https://github.com/akarnokd/rxjava2-backport/releases/tag/2.0.0-RC1

It includes a bunch of goodies of recent PRs.

@akarnokd
Copy link
Member

Java 8 language level remain in use for source files

This would might not necessary. I was a bit sloppy and used create() with lambdas, but I really should have put those into real classes inside internal. Otherwise, Eclipse can mostly transform a lambda expression in a Java 6 project to inner class representation.

There's two APIs which are currently used in a high-leverage way:
ScheduledThreadPoolExecutor.setRemoveOnCancelPolicy(boolean)

Using purge within a periodic background thread. There is no version check so setRemoveOnCancelPolicy is not used.

StampedLock

Switched back to read-write lock which does some extra atomics for each Subscriber if it happens to be racing with an emission. (A backport, unfortunately drags in a bunch of dependent classes and unsafe atomics.)

Optional

Using a limited backport, but it may go away in all releases because of the increased allocation (1 Try, 1 Optional) for a materialized notification value.

CompletableFuture, Stream

I just dropped those methods that were using these.

java.util.function.*

Backported them into the functions package + internal.functions.Functions helper.

java.util.Objects and other extra methods

Implemented the methods in internal.functions.Objects helper.

The Animal Sniffer tool can be used to automate detection of Java 8 APIs in use, and enforce at compile-time that none are used.

Did not use this tool. I developed the backport via a v6 JDK (avoid accidental API use), once worked, swapped in a v8 JDK and fixed all name conflicts.

j.u.c.Flow

This requires replacing org.reactivestreams.* only.

value types

This requires a separate instance and wouldn't work with reactive-streams as one has to declare <any T> fairly deep and use it everywhere.

If JDK 8 API is not a requirement for RxJava v2, probably neither should be a requirement of Java 8 lang syntax.

It made v2 development super easy and the backporting super annoying.

Benefits of using Java 8 lang features seem low for a library.

v2 uses many Java 8 conveniences, nothing that couldn't be cast away.

What is the plan for 1.x going forward?

There will be a 1.0.15 release eventually, and after that 1.1.0 without any API change (but perhaps with bugfixes).

Is there dual support planned, or is the future going to consist mostly of 2.x+?

In theory, a dual support was planned, which now may end up with tri-support. Bugfixes may have to be applied to all 3 versions and feature requests may end up in all 3.

Frankly, I believe v1 support should be reduced to patch releases as soon as the ecosystem built around it creates their v2 implementations.

One can debate if such libraries should do Reactive-Streams-based public API or directly an RxJava based, but they can't support v2-java8 and v2-java6 at the same time I guess. RS is a safer choice but then everyone has to call fromPublisher all the time. The alternative is to return a FluentPublisher that extends RS Publisher with a to method for fluent conversion.

@JakeWharton
Copy link
Contributor Author

Your separate backport is interesting, but seems untenable and isn't really the focus of this issue.

they can't support v2-java8 and v2-java6

This issue is not proposing the creation of separate releases, but changing 2 to simply work on Java 6 and up.

@dlew
Copy link

dlew commented Oct 16, 2015

How will we guarantee that the backport will have the same code as the original?

I've maintained dual libraries before. It's not fun and it's error prone.

@akarnokd
Copy link
Member

How will we guarantee that the backport will have the same code as the original?

Since we have to leave out API with CompletableFuture and Stream I think it is not possible. We can, of course, make v2 default to Java 6 and put all extras into some external library. In addition, the scheduler purging requires customization between the two targets or a fallback to the v1 reflective way.

It's not fun and it's error prone.

I already "have to" maintain 2 versions and 5-7 components, i.e., v1 Observable, v1 Single, v1 Completable?, v2 Observable, v2 NbpObservable, v2 Single, v2 Completable? Not maintaining +4 components would certainly a relief to me. I'm fine with replacing the current 2.x with the backported variant but I've serious doubts about when anything official comes out from this project lately. So yes, we might have one "official" version that nobody gets or two versions of which one gets out to developers.

@benjchristensen
Copy link
Member

If we can maintain the code with lambdas, that would be preferable, as the code between 2.x and 3.x would then be a clean increment to just adopting Java 8/9 APIs, rather than changing all the code.

If we make 2.x support Android, then 1.x would move towards end-of-life far sooner than the current plan which has been to support 1.x long-term, while 2.x targeting JDK 8+ only.

@JakeWharton I want to explore your approach a little more as it seems the most tenable for maintenance.

Let's say we do 2.x with lambdas (like the code that currently exists in the 2.x branch).

  1. Is Retrolambda trustworthy enough to use it as a "production-grade" solution for everyone to rely upon the resulting Jars?
  2. If we kept to basic JDK 8 API usage (confirmed via build rules), for things like Stream, CompletableFuture and java.util.function.*, is it okay to write ASM rules to remove those from a Jar targeting Android so that we don't have to remove them from 2.x completely and affect normal Oracle JVM usage?
  3. Is it okay to have a separate artifact for Android (such as rxjava-android, or perhaps just rxandroid) that is the result of steps 1 and 2?

If we can do these things, I think it would mean we would not need to have a 3.x that then targets Java 8/9, but can work off of just the 2.x branch for both, which would be far better.

@akarnokd
Copy link
Member

Posted PR #3460 that adds purge/removeOnCancelPolicy settings and support.

@JakeWharton
Copy link
Contributor Author

Is Retrolambda trustworthy enough to use it as a "production-grade" solution for everyone to rely upon the resulting Jars?

Retrolambda now seems very stable and reliable. I was initially very hesitant, but after a year of watching it mature in features and reliability and a deep audit of it's behavior I'm very confident in it.

If we kept to basic JDK 8 API usage (confirmed via build rules), for things like Stream, CompletableFuture and java.util.function.*, is it okay to write ASM rules to remove those from a Jar targeting Android so that we don't have to remove them from 2.x completely and affect normal Oracle JVM usage?

As long as these code paths are not executed, they do not have to be removed. You can write non-overloaded* APIs that take JDK 8 types and you can write classes that even use JDK 8 types provided that none of the bytecodes actually execute. Since Android cannot link against these methods (and thus run them), as long as the types are not used internally elsewhere and only for satisfying those methods there should be no changes needed.

*Basically raw overloads do not work (e.g., foo(OldType) and foo(NewType)) but overloads with different resolution semantics such as argument count do work (e.g., foo(OldType) and foo(NewType, OtherType)) as well as just different names (foo(OldType) and bar(NewType)).

Is it okay to have a separate artifact for Android (such as rxjava-android, or perhaps just rxandroid) that is the result of steps 1 and 2?

As I said I don't think we need 2, but to answer your question yes it would be okay, but not ideal.

Ideally the main artifact would retain Java 6 compatibility. This would be a huge win to Android, a community that really has stepped up to embrace this library (</shamlessHeartStringPulling>). Whether this is through Retrolambda-ing the main artifact, or just sucking it up and backing out lambdas and method references, the Android community would seriously appreciate this. We absolutely need the backpressure distinction of Observable and Flowable in the type system and would love to write, share, and use libraries built against those APIs without the worry of binary incompatibility. A separate artifact without extreme care will introduce incompatibilities between the two (bad) and the backport in a different package solution is needless fragmentation which allows zero interop (worse).

It sucks, and it sucks being on the side that has to beg for this all over the place. Give us 2.x on Android, and if we're not at least able to see the light of Java 8 when 3.x rolls around it'll be Google receiving our wrath, not projects like this which deserve to move on in their compatibility story.

@sdeleuze
Copy link

What about using the Streamsupport backport in addition to Retrolambda to achieve Android compatibility for RxJava 2?

It has been recently split in 4 parts:

  • streamsupport: java.util.function (functional interfaces) and java.util.stream (streams)
  • streamsupport-cfuture: CompletableFuture API
  • streamsupport-atomic: j8.u.c.atomic package
  • streamsupport-flow: Java 9 Flow API

@JakeWharton
Copy link
Contributor Author

CompletableFuture is only referenced for adapting the platform type so it wouldn't make sense to use a backport. Streams are only used as a convenience in two or three places and their usage is trivially replaced with equivalent code that doesn't use a stream.

I don't see what value a functional interface backport provides over just expanding the interfaces provided in the library. The latter gives you more semantic naming and using methods accepting these types on Java 8 is going to let you invoke them with any compatible type anyway.

I would imagine the needed atomics would either be copied into the internal package or swapped for something of equivalent functionality.

In general, RxJava has always been strong on being free from transitive dependencies. It destabilizes the compatibility guarantees the library provides. The JCTools collections in use are manually shaded in the internal package, for example.

@stevegury
Copy link
Member

The android developers are a big part of the RxJava community, I would prefer having the 2.0 compatible with Android to ease the migration.
retrolambda seems now widely accepted as a stable project, I'm not sure about streamsupport though.

@akarnokd I didn't know about your backport https://github.com/akarnokd/rxjava2-backport, would you consider merging it in the 2.x branch, or what do you think if we use retrolambda?

@JakeWharton
Copy link
Contributor Author

Streamsupport isn't needed. The internal uses of streams are trivially
replaceable with imperative versions. Uses in the public API can be put in
an RxJava2-Java8 project which provides adapters and converters for
platform types.

On Wed, Feb 3, 2016, 7:03 PM Steve Gury notifications@github.com wrote:

The android developers are a big part of the RxJava community, I would
prefer having the 2.0 compatible with Android to ease the migration.
retrolambda seems now widely accepted as a stable project, I'm not sure
about streamsupport though.

@akarnokd https://github.com/akarnokd I didn't know about your backport
https://github.com/akarnokd/rxjava2-backport, would you consider merging
it in the 2.x branch, or what do you think if we use retrolambda?


Reply to this email directly or view it on GitHub
#3450 (comment).

@stevegury
Copy link
Member

@JakeWharton completely agreed.

@akarnokd
Copy link
Member

akarnokd commented Feb 4, 2016

See #3668.

@stevegury
Copy link
Member

Great! Thanks @akarnokd !

@ZacSweers
Copy link
Contributor

Can this be closed now?

@stevegury
Copy link
Member

Closing per #3668.

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

9 participants