vavr-io / vavr

vÊŒvr (formerly called Javaslang) is a non-commercial, non-profit object-functional library that runs with Java 8+. It aims to reduce the lines of code and increase code quality.
https://vavr.io
Other
5.76k stars 637 forks source link

Spike: modularization #2228

Closed danieldietrich closed 6 years ago

danieldietrich commented 6 years ago

Yesterday I bootstrapped a new, modularised version of Vavr. It runs on Java 9 and will be the base for the upcoming 1.0.0 release.

In this issue I want to collect early feedback while tracking the migration from the previous code base Vavr 0.9.x to the next code base Vavr 1.0.0.

The Vavr 1.0.0 source can be found in the v1.0.0 branch.

Log

Gradle module Java 9 module exports requires transitive
vavr (root)
vavr-core io.vavr.core io.vavr.core
vavr-control io.vavr.control io.vavr.control io.vavr.core
vavr-collection io.vavr.collection io.vavr.collection io.vavr.control
vavr-collectionx io.vavr.collectionx io.vavr.collectionx io.vavr.collection
vavr-concurrent io.vavr.concurrent io.vavr.concurrent io.vavr.control
vavr-api io.vavr.api io.vavr.api io.vavr.collectionx, io.vavr.concurrent
talios commented 6 years ago

On 19 Mar 2018, at 12:46, Daniel Dietrich wrote:

Yesterday I bootstrapped a new, modularised version of Vavr. It runs on Java 9 and will be the base for the upcoming 1.0.0 release.

Is this to say, it ONLY runs on Java 9 (and higher)? So no Java 8 support, or is that still available?

Currently we hide impls of Either (Left and Right) in a subpackage called internal. Is this idiomatic Java 9?

I've not really seen anyone talking about idiomatic design for Java 9/jigsaw worlds, but that's the kind of thing I'd do under OSGi.

And part of what I've been pondering lately regarding that tweet of mine ( and flowing onto a reddit discussion I started at https://www.reddit.com/r/java/comments/859i84/thoughts_on_large_class_apis_using_defaultstatic/ )

Cheers Mark


"The ease with which a change can be implemented has no relevance at all to whether it is the right change for the (Java) Platform for all time." — Mark Reinhold.

Mark Derricutt http://www.theoryinpractice.net http://www.chaliceofblood.net http://plus.google.com/+MarkDerricutt http://twitter.com/talios http://facebook.com/mderricutt

emmanueltouzery commented 6 years ago

Hmm i don't know about hiding Left and Right. It will make the apidoc cleaner, but it may become a hindrance if/when java gets pattern matching ( http://cr.openjdk.java.net/~briangoetz/amber/pattern-match.html ).

It depends how you go with methods which would fit only on Right. For instance Either could have getLeft and getRight both returning Option or getLeftOrThow and getRightOrThrow, while Left could have getLeft returning L and Right could have getRight returning R. So pattern matching would ensure safe calls.

But we could always revisit that if/when java does get this pattern matching. Hide now and add back then maybe. Currently java/vavr instanceof matching is not very idiomatic. In prelude.ts Either Either has getLeftOrElse, getLeftOrThrow, getOrElse, getOrThrow. Right has get. Left has getLeft.

Opalo commented 6 years ago

I've looked around a bit and it seems that internal is something that makes sense in java 9 world. Whether it's internal or any other name doesn't make difference - it's a (sub)package that isn't exported via module-info.java.

@talios mentioned quite important matter - what about java 8 support?

SergejIsbrecht commented 6 years ago

Would like to see Java8 supported as well. As already discussed on twitter, it could probably be accomplished with https://blog.gradle.org/mrjars . I could have a look into it.

yuriykulikov commented 6 years ago

Hello, is not a little bit too early for Java 8? If Java 8 support is dropped, it will effectively mean that I will not be able to use Vavr in any of my projects (private and at work). Android is likely to be stuck on Java 8 for a long time.

danieldietrich commented 6 years ago

I agree, supporting Java 8 should be the goal, especially because we do not use any Java 9 features besides module-info.java.

@SergejIsbrecht Java 8 is important for most Vavr users! Thx for volunteering, I'm already on it.

@talios @yuriykulikov@Opalo thx! ok, understood :) I will figure out how to support both Java 8 and native modules. But remember, the Vavr 1.0 API will break backward compatibility in several places.

However, when we support Java 8, the internal package will leak into the public API. We could move them to the parent package and make them package-private. But I don't know if we are able to hide them from the Java 9 module public API then. (Our modules are closed for reflection, but I haven't tested it, yet).

@emmanueltouzery

but it may become a hindrance if/when java gets pattern matching

Adding them later is possible without braking bw compat. I want to wait for the final patmat feature. Maybe we need to change the impls in order to get patmat working with Vavr.

danieldietrich commented 6 years ago

I've thought about our options.

Building binaries for both Java 8 and Java 9

We don't use any other features from Java 9 than the module system. Our main-use case is not exposing the modules to the outside, this is a positive side-effect. The main use-case of adapting Jigsaw is to have a clean architectural design of our inter-package dependencies. More specifically, we benefit from Jigsaw during development time.

Because we 1) only use Java 8 API and 2) effectively only have an additional module-info.java of Java 9, we are able to produce Java 8 and Java 9 binaries from the same sources. Now, here we have two options:

  1. Create multiple versions of each jar, one for Java 8 and one for Java 9 (by using classifiers)
  2. Create multi-release jars, which contain Java 8 classes plus an additional module-info.class for Java 9+

The Gradle team looks critically at multi-release jars. This is because it is hard to reason about the behavior of an application when different versions (and different dependencies) are bundled at the same time. With multi-release jars the gap between source-code, modules and artifacts gets bigger.

However, we are in the comfortable situation that for both runtimes, Java 8 and Java 9, we use exactly the same source-code. Only the module-info.class is additionally used by the Java 9 runtime. Therefore I think a multi-release jar is (currently) the perfect solution for us. I don't see any drawbacks.

I will start to update the Gradle build accordingly.

Hiding implementation classes

Having an internal package in Java 9 that is not exported by the module solves the issue of hiding classes. However, putting the classes in a separate package (and making them therefore public) makes most sense when the classes are referenced from multiple packages.

In Java 8 such an internal package would leak the implementation classes to the outside, because they need to be public.

Therefore I think it is a better solution, to put make the implementation classes package-private and put them directly into the package of their interfaces/abstract classes. This will solve our original use-case of hiding the classes for both, Java 8 and Java 9.

Especially (I think, but have to re-evaluate it) these package-private classes are not discoverable via reflection in Java 9, if the module isn't declared as open.

danieldietrich commented 6 years ago

I need a helping hand with this: Multi-Release JAR that contains Java 8 binaries + Java 9 module-info.class. /cc @SergejIsbrecht

talios commented 6 years ago

On 20 Mar 2018, at 11:31, Daniel Dietrich wrote:

  1. Create multi-release jars, which contain Java 8 classes plus an additional module-info.class for Java 9+

I've not actually created any Jigsaw modules yet, but I think you can have a non-MRJ with module-info.class in it, and still work in Java 8 - since it's not a valid class name, nothing in Java can actually refer to the class to load it, but java 9 will use it.

So it might be possible to just have a compile step for module-info.java that uses -release 9 and sticks it in the normal target/classes directory for jaring up?

Interesting, that post you link to ( which I was already reading ) is only on MRJs, not Modules-with-MRJs - does module-info.class need to be in / or can it be in META-INF/versions/9 as well?

Mark

"The ease with which a change can be implemented has no relevance at all to whether it is the right change for the (Java) Platform for all time." — Mark Reinhold.

Mark Derricutt http://www.theoryinpractice.net http://www.chaliceofblood.net http://plus.google.com/+MarkDerricutt http://twitter.com/talios http://facebook.com/mderricutt

SergejIsbrecht commented 6 years ago

@danieldietrich, will have a look at it after work today

Opalo commented 6 years ago

In Java 8 such an internal package would leak the implementation classes to the outside, because they need to be public.

With internal package you can always warn the end users that this package's content may change at any time and you're not responsible for these changes 😉 Of course it doesn't make much sense and even though such packaging (I mean internal) is quite common it's a leaky abstraction and don't like it much.

Therefore I think it is a better solution, to put make the implementation classes package-private and put them directly into the package of their interfaces/abstract classes. This will solve our original use-case of hiding the classes for both, Java 8 and Java 9.

This (^) sounds really good. I'm wondering only if this is possible for all classes - no implementation will leak.

Also, I know that Java 8 and 9 builds have exactly the same source right now hence mrjar can be used. Will it change in foreseeable future? If so maybe it's better idea to prepare separate builds now and save some time in the future. No idea if this is a good suggestion, rather asking 😉

Let me know if you need any help with gradle.

danieldietrich commented 6 years ago

@SergejIsbrecht great!

@Opalo

I'm wondering only if this is possible for all classes

We already do this for internal classes in Vavr 0.9. However, in Java 8 they are still discoverable using reflection. Namely changing internal classes might break backward compatibility. Java 9 modules to the rescue!

If so maybe it's better idea to prepare separate builds now and save some time in the future

In my experience this would be too early optimization. We currently do not need multiple versions of classes. This would add unnecessary complexity. We will add them if they are really needed.

Thx for all your thoughts and the feedback!

danieldietrich commented 6 years ago

@SergejIsbrecht for now I fell back to a Java 8 only build. Please do not spend too much time on the Java8/Java9 multi-release jar. I think it might not work well for us or the build gets too complicated. For now a good old Java 8 build should be sufficient.

SergejIsbrecht commented 6 years ago

Ok, I will look into the suggestion of melix to generate binaries for each java version: java8 / java9 / java10. Probably use target Java10, because Java9 is not supported anymore. It would be great if we could hide internal implementation for >= Java9 and use internal package for Java8. I will play around a little bit and will give you feedback on this issue.

danieldietrich commented 6 years ago

Vavr 1.0 runs with Java 11+. I switched the master branch, it now contains the first module: vavr-control v1.0.0-alpha-1. More will follow soon (e.g. vavr-collection).

Java 8 will reach its end of life soon. Legacy applications are still able to use Vavr 0.9.x.

The original purpose of this issue (creating a spike) is done.