Added java6 project using retrolambda

[rdegnan]

Since #3668 was merged we should now be able to build a separate artifact for Java 6, while continuing to maintain the main project using Java 8. This splits the project into 'rxjava' and 'rxjava-java6', which compiles the same source code using retrolambda, and uses the animalsniffer plugin to ensure that we are not inadvertently using APIs introduced after Java 6.

[akarnokd]

I don't think it's worth it. RxJava 2.x will be Java 6 based and the Stream-API interoperation has to go into a separate project; or just use somebody else's Stream -> Publisher wrapper.

[akarnokd]

Vote for closing as unnecessary. RxJava 2.x targets Java 6 and we can live with the small inconvenience of using inner classes.

[abersnaze]

@akarnokd @artem-zinnatullin: I'm not sure if it is only for convenience. Wouldn't there be a performance improvement by being able to use invoke dynamic internally?

@rdegnan: Would it publish the jvm version in the classifier part of the artifact (rxjava-2.0.0-java8.jar)?

[artem-zinnatullin]

Most or probably all use cases for invokedynamic in RxJava will be lambdas.

Such usage of invokedynamic is currently implemented (in the HotSpot at least, afaik) pretty much the same as compiling to anonymous class but at runtime: both from performance point of view and implementation (afaik it generates same anonymous class but at runtime).

(Afaik) invokedynamic gives noticeable performance improvements for cases when you need to do some dynamic dispatch widely used in dynamic JVM languages like Groovy, but I don't think we need it in RxJava.

// Will try to invite some engineers with better knowledge of JVM/JDK to discussion.

[akarnokd]

Internally, we have many context-capturing inner classes so I think lambdas would end up as object instances anyway and the runtime JIT is mostly capable of inlining through regular inner classes.

[stealthcode]

@akarnokd could you try running your JMH test suite with java6 and java8?

[rdegnan]

We can set it up to use classifiers, that would be better — right now it’s just appending ‘-java6’ to the name.

In addition to removing thousands of lines of Java boilerplate, I think there are advantages to having a build that targets Java 8 in addition to a build that targets Java 6 via retrolambda. One problem is that methods like BlockingObservable.forEach are inconvenient to use in Java 8 because BlockingObservable also inherits forEach(java.util.function.Consumer) from Iterable, so using a lambda is not possible because it is ambiguous whether it should implement java.util.function.Consumer or io.reactivex.functions.Consumer. In the Java 8 build io.reactivex.functions.Consumer extends java.util.function.Consumer so this is not an issue. In addition we would be able to take advantage of things like default methods on io.reactivex.functions.* which can make functional programming in Java a lot more convenient (see https://github.com/javaslang/javaslang https://github.com/javaslang/javaslang).

I’m also curious about the performance improvement in practice, though I’m not sure which perf test would best reflect the difference.

On Mar 23, 2016, at 4:13 PM, George Campbell notifications@github.com wrote:

@akarnokd https://github.com/akarnokd @artem-zinnatullin https://github.com/artem-zinnatullin: I'm not sure if it is only for convenience. Wouldn't there be a performance improvement by being able to use invoke dynamic internally?

@rdegnan https://github.com/rdegnan: Would it publish the jvm version in the classifier part of the artifact (rxjava-2.0.0-java8.jar)?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #3764 (comment)

[JakeWharton]

Those sound like binary incompatible features between the hypothetical
classified artifacts which would destroy the Rx ecosystem's crossover with
Android rendering the elaborate setup mostly meaningless.

On Wed, Mar 23, 2016, 7:44 PM Ryland Degnan notifications@github.com
wrote:

We can set it up to use classifiers, that would be better — right now it’s
just appending ‘-java6’ to the name.

In addition to removing thousands of lines of Java boilerplate, I think
there are advantages to having a build that targets Java 8 in addition to a
build that targets Java 6 via retrolambda. One problem is that methods like
BlockingObservable.forEach are inconvenient to use in Java 8 because
BlockingObservable also inherits forEach(java.util.function.Consumer) from
Iterable, so using a lambda is not possible because it is ambiguous whether
it should implement java.util.function.Consumer or
io.reactivex.functions.Consumer. In the Java 8 build
io.reactivex.functions.Consumer extends java.util.function.Consumer so this
is not an issue. In addition we would be able to take advantage of things
like default methods on io.reactivex.functions.* which can make functional
programming in Java a lot more convenient (see
https://github.com/javaslang/javaslang <
https://github.com/javaslang/javaslang>).

I’m also curious about the performance improvement in practice, though I’m
not sure which perf test would best reflect the difference.

On Mar 23, 2016, at 4:13 PM, George Campbell notifications@github.com
wrote:

@akarnokd https://github.com/akarnokd @artem-zinnatullin <
https://github.com/artem-zinnatullin>: I'm not sure if it is only for
convenience. Wouldn't there be a performance improvement by being able to
use invoke dynamic internally?

@rdegnan https://github.com/rdegnan: Would it publish the jvm version
in the classifier part of the artifact (rxjava-2.0.0-java8.jar)?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub <
https://github.com/ReactiveX/RxJava/pull/3764#issuecomment-200577953>

—
You are receiving this because you are subscribed to this thread.

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

[akarnokd]

Sorry, I don't have the capacity to experiment with various runtime versions right now. Most inner classes get instantiated during the assembly time thus you'd only see differences with trivial functions on short sequences.

In addition, 2.x started out as a native Java 8 code and I didn't really need any Java 8 features inside the operators. It was more of a convenience in unit tests but Eclipse can turn a lambda into an inner class in a JDK 6 targeted project most of the time.

BlockingObservable.forEach could be renamed to something non-conflicting, but I think who uses BlockingObservable should pay the price of manual casting to disambiguate.

[stealthcode]

@JakeWharton the purpose of this is to have a single source code that generates 2 different artifacts. Android users would (by default) get the java6 compatible version. For those who want to use the java8 version they could change the dependency to point to java8 classifier.

@akarnokd you want to close this PR without looking at performance metrics?

[artem-zinnatullin]

@stealthcode but how other libraries will depend on RxJava then if we will have 2 artifacts…?

[JakeWharton]

Right. The artifacts need to be binary compatible for libraries to work
against both. And if binary compatibility is broken there's no point in
making two artifacts because they effectively become two separate,
incompatible libraries.

On Wed, Mar 23, 2016, 8:06 PM Artem Zinnatullin notifications@github.com
wrote:

@stealthcode https://github.com/stealthcode but how other libraries
will depend on RxJava then if we will have 2 artifacts…?

—
You are receiving this because you were mentioned.

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

[stealthcode]

@artem-zinnatullin here is a gradle example of how dependencies would be expressed.

Java6 Project build.gradle

compile 'io.reactivex:rxjava:1.1.2' // java 1.6 compatible

Java8 Project build.gradle

compile 'io.reactivex:rxjava:1.1.2:jdk18' // java 1.8 compatible

// alternate
compile group: 'io.reactivex', name: 'rxjava', version: '1.1.2', classifier: 'jdk18'

[stealthcode]

@JakeWharton if a library depends on the RxJava java8 version then it's runtime should be 1.8 and thus all usages of that library should also be running on 1.8. Why would a library need to be binary compatible with both versions?

[artem-zinnatullin]

@stealthcode please clarify if end user project will be able to specify which RxJava artifact it'll need: jdk8 or jdk6 even if some library that it depends on needs artifact with another classifier (until they're binary compatible, of course).

Example:

• lib1 depends on RxJava-2-jdk8
• proj1 depends on RxJava-2-jdk6 and lib1

[stealthcode]

@artem-zinnatullin in that situation the build would include both artifacts on the classpath and proj1 would of course not compile. So as an owner of proj1 it would not make sense to use a library designed for use with java8. However ultimately it's up to the library owner if they chose to depend on the binary incompatible version of rxjava. IMO a library author in the public domain should publish their artifact building against the standard 1.6 compatible rxjava or if they really want to they could publish 2 versions with a classifier or separate artifacts.

However if you can predict that all users of your library specifies targetCompatibility = 1.8 then the library author could make that decision safely. E.g. a library for consumption in a companies infrastructure where they know that all consuming projects will be running on java8 could safely publish with a dependency on the jdk18 classifier.

[JakeWharton]

Retrofit needs to be compatible with both because it works on both Android
and Java 8. Along with hundreds of other libraries targeting both.

On Wed, Mar 23, 2016, 9:03 PM Aaron Tull notifications@github.com wrote:

@artem-zinnatullin https://github.com/artem-zinnatullin in that
situation the build would include both artifacts on the classpath and
proj1 would of course not compile. So as an owner of proj1 it would not
make sense to use a library designed for use with java8. However ultimately
it's up to the library owner if they chose to depend on the binary
incompatible version of rxjava. IMO a library author in the public domain
should publish their artifact building against the standard 1.6 compatible
rxjava or if they really want to they could publish 2 versions with a
classifier or separate artifacts.

However if you can predict that all users of your library specifies targetCompatibility
= 1.8 then the library author could make that decision safely. E.g. a
library for consumption in a companies infrastructure where they know that
all consuming projects will be running on java8 could safely publish with a
dependency on the jdk18 classifier.

—
You are receiving this because you were mentioned.

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

[JakeWharton]

You actually are actively making things worse by using the same package
name and Maven coordinates for two binary incompatible artifacts. If you
are breaking binary compatibility which I am violently against then change
the package name and artifactId.

On Wed, Mar 23, 2016, 9:12 PM Jake Wharton jakewharton@gmail.com wrote:

Retrofit needs to be compatible with both because it works on both Android
and Java 8. Along with hundreds of other libraries targeting both.

On Wed, Mar 23, 2016, 9:03 PM Aaron Tull notifications@github.com wrote:

@artem-zinnatullin https://github.com/artem-zinnatullin in that
situation the build would include both artifacts on the classpath and
proj1 would of course not compile. So as an owner of proj1 it would not
make sense to use a library designed for use with java8. However ultimately
it's up to the library owner if they chose to depend on the binary
incompatible version of rxjava. IMO a library author in the public domain
should publish their artifact building against the standard 1.6 compatible
rxjava or if they really want to they could publish 2 versions with a
classifier or separate artifacts.

However if you can predict that all users of your library specifies targetCompatibility
= 1.8 then the library author could make that decision safely. E.g. a
library for consumption in a companies infrastructure where they know that
all consuming projects will be running on java8 could safely publish with a
dependency on the jdk18 classifier.

—
You are receiving this because you were mentioned.

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

[rdegnan]

Another option which would avoid having to use classifiers is to build rxandroid as a subproject alongside rxjava. As I have done here, it would compile the same sourceset plus the additional code that currently lives in the rxandroid repo, targeting java 6. RxAndroid would have no dependency on RxJava, the artifacts would be different and there is no reason people would want both. We would also know instantly if any changes to RxJava break android.

On Mar 23, 2016, at 6:16 PM, Jake Wharton notifications@github.com wrote:

You actually are actively making things worse by using the same package
name and Maven coordinates for two binary incompatible artifacts. If you
are breaking binary compatibility which I am violently against then change
the package name and artifactId.

On Wed, Mar 23, 2016, 9:12 PM Jake Wharton jakewharton@gmail.com wrote:

Retrofit needs to be compatible with both because it works on both Android
and Java 8. Along with hundreds of other libraries targeting both.

On Wed, Mar 23, 2016, 9:03 PM Aaron Tull notifications@github.com wrote:

@artem-zinnatullin https://github.com/artem-zinnatullin in that
situation the build would include both artifacts on the classpath and
proj1 would of course not compile. So as an owner of proj1 it would not
make sense to use a library designed for use with java8. However ultimately
it's up to the library owner if they chose to depend on the binary
incompatible version of rxjava. IMO a library author in the public domain
should publish their artifact building against the standard 1.6 compatible
rxjava or if they really want to they could publish 2 versions with a
classifier or separate artifacts.

However if you can predict that all users of your library specifies targetCompatibility
= 1.8 then the library author could make that decision safely. E.g. a
library for consumption in a companies infrastructure where they know that
all consuming projects will be running on java8 could safely publish with a
dependency on the jdk18 classifier.

—
You are receiving this because you were mentioned.

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

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #3764 (comment)

[JakeWharton]

the artifacts would be different and there is no reason people would want both

Except, again, the libraries that support both. You've basically just pushed the problem and burden of making two versions of every library or having two integrations on the hundreds of libraries that want to support both.

This isn't acceptable.

[artem-zinnatullin]

@stealthcode then it I'll basically kill development of generic libraries around RxJava that target both java 6 and java 6+ like rxjava-async-util, Retrofit, many db drivers and so on.

Whole community will be confused and everybody will blame us for such decision.

I'm sorry, but I'm voting against splitting library into two artifacts.

[akarnokd]

I'm not an expert on how and when lambdas are converted to instances, but I'd expect whenever a lambda ends up in a field, it has to be instantiated.

I did a search for "new Function" and they are only in the base classes mostly part of the convenience overloads:

Flowable:

• zip(Publisher<Publisher>>, Function) - depends on an outer parameter
• cast(Class) - alias for map with function that casts
• concatMapIterable(Function) - delegates to concatMap with an inner function depending on the outer parameter
• delay(Function) uses flatMap and complex inner function depending on the outer parameter + some inner capture of a variable
• delaySubscription(time) uses flatMap and a capturing lambda for the outer this
• delaySubscription(Flowable) similar to the previous
• flatMap(Function, BiFunction) two functions depending on different levels one up
• flatMapIterable(Function) delegates to flatMap with an inner function depending on the outer parameter
• onErrorResumeNext(Flowable) uses the function variant with a function returning the Flowable
• onErrorReturnValue uses onErrorReturn with a function returning the value
• onExceptionResumeNext(Flowable) uses the function variant with a function returning the Flowable
• repeatWhen one function depending on the handler and a deeper function which is constant (can be factored out)
• replay(Function, int, Scheduler) calls another replay with functions capturing the outer parameters
• repeatWhen a function depending on the outer parameter and two pure functions (can be factored out)
• timestamp function depends on the outer parameters
• toMultimap variants with pure functions (can be factored out)
• toSortedList(Comparator) with a function depending on the comparator

Functions
Conversions 2-9 parameter functions to Function<Object[], R> wrapper that end up in a field.

Observable
Roughly the same as with Flowable

Single

• concat delegates to concatMap with a pure function (can be factored out)
• merge delegates to flatMap with a pure function (can be factored out)

Besides, I remember the time when we had all sorts of subprojects and changing some Java bits ended up breaking the compilation or tests of the other subprojects with no one around to help fix those. It is already cumbersome to keep track the features and tests in both 1.x and 2.x and I don't want to do more maintenance than necessary. I also agree with @JakeWharton and @artem-zinnatullin.

[stevegury]

I've recently read about invokedynamic to understand more about the problem, and if my understanding is correct, here are my thoughts:

The main advantage of targeting java 8 (i.e. ByteCode v52) is the ability to use the invokedynamic instruction. On Java 8, the jvm defers the translation of lambdas at runtime (thanks to invokedynamic), it can translate the lambda into something like an anonymous class, a method reference or other. This can lead to more efficient code and save allocations.

Note that invokedynamic is used in the code that creates the lambda, but not where the lambda is invoked. Most of the usage of new Func* that I saw in Observable.java don't capture anything and would benefit from indy.

I agree with @JakeWharton that if we decide to have to artifacts, the binary compatibility MUST be respected (and tested). Otherwise, multiple transitive dependencies may cause NoSuchMethodException.

I think that if the build tool is properly setup, it shouldn't cause any problem to anyone. Worst case would be, having two transitive dependencies on RxJava 2, one jdk6 and one jdk8, depending on the classpath order, the jvm would either load the jdk6 bytecode (and not benefit from indy) or load jdk8 bytecode (and benefit from indy).

[stealthcode]

Otherwise, multiple transitive dependencies may cause NoSuchMethodException.

The artifacts would be based on the same source code. I don't think that you could get a NoSuchMethodException or a ClassNotFoundException. I'm also pretty sure that byte code call sites from user code (or library code) to RxJava would not need to recompile if they depended on 1.8 or 1.6. For example

proj_library depends on RxJava:jdk18
proj1 depends on RxJava (the default 1.6 + Retrolambda build) and proj_library

If proj1 excluded RxJava:jdk18 in the build script then both proj_library and proj1 would be running against the 1.6 compatible code. This should still function as the call sites from both should not have to be recompiled.

[benjchristensen]

I strongly feel that there should be a single artifact. With Java 9 (which is not that far off) we can then use multi-version JARs to target Java 9+ enhancements on top of the Java 6+ base (such as adding support for all the Java 8+ and 9+ APIs).

As for the performance improvements lambdas can offer, I'm pretty certain that if we have code where the perf benefits from a lambda, we can stop using anonymous inner classes and define static inner classes, or use static functions instead and get the same improvements.

If we are "newing" up an Object such as a Func, then it has instance state and a lambda is not going to dramatically improve anything if at all. Lambdas are dramatically better than anonymous inner classes when it can be determined that they are static and only get instantiated once and invoked many times ... just like we can do manually with statics.

[akarnokd]

Thanks for the contribution.

Since the decision has been made to go for Java 6, I don't see any need or value doing Java 8 sources and cross-compiling it. Certainly, some unit tests would love the simplified lambda syntax but the IDEs can usually fix the "syntax error" of a lambda expression.

Java 9 is a bit far out and until the usual tools get updated, I can't comment on how the multi-versioned jars would work (out).

What I definitely want is separate id for the 2.x. Let's discuss that further in #3170