Release JDK 13 compatible version

[davido]

According to ASM page: [1], ASM 7.1 version is required to support Java 13. In 74304f9, ASM was upgraded to 7.1 already, but a new release wasn't conducted yet.

[1] https://asm.ow2.io/versions.html

[cogman]

Would it be possible to stop shading asm and cglib? You can often bump those up without causing breaks. It will be annoying to have to wait for guice to release every 6 months when new java versions come out.

[brharrington]

@cogman see previous discussion in #1198.

[mcculls]

@cogman the build already produces a jar that doesn't bundle asm or cglib:

https://repo1.maven.org/maven2/com/google/inject/guice/4.2.2/guice-4.2.2-classes.jar

This is a jar of the Guice classes before any JarJar'ing which you can then bundle yourself with a particular version of asm and cglib.

Also if you don't need AOP then there's the "no_aop" jar which doesn't use asm/cglib at all:

https://repo1.maven.org/maven2/com/google/inject/guice/4.2.2/guice-4.2.2-no_aop.jar

[davido]

@mcculls Thanks for pointing this out. Unfortunately, no_aop is not a standalone artifact and cannot be easily consumed? In Gerrit we are using Bazel build tool, and I was able to fetch this custom guice artifact, but the CL is a bit hacky: [1], here is the relevant part:

http_file(
    name = "guice-library-no-aop",
    downloaded_file_path = "guice-library-no-aop.jar",
    sha256 = "0f4f5fb28609a4d2b38b7f7128be7cf9b541f25283d71b4e56066d99683aafff",
    urls = [
        "https://repo1.maven.org/maven2/com/google/inject/guice/" +
        GUICE_VERS +
        "/guice-" +
        GUICE_VERS +
        "-no_aop.jar",
    ],
)

[1] https://gerrit-review.googlesource.com/c/gerrit/+/243014

[GedMarc]

I've released the fully modular guice artifacts based on 4.2.2
All extensions as well

0.70.0.1 is busy going to maven central

guice : https://search.maven.org/artifact/com.guicedee.services/guice/

guice extensions all available -

Current site https://guicedee.com/ - Done the pages up to persistence, should have it all done by the weekend.

[davido]

I've released the fully modular guice artifacts based on 4.2.2

How is your new distribution is different from the official distribution, except the different GAV? The point was to have Maven coordinates for a Guice distribution for NO-AOP flavor, without shaded ASM to not mess around with transitive dependencies and with the fact that newer JDK version is released every couple of months now, whereas the new Guice version is released only every couple of years (if at all).

IOW, what I am interesting in, is to be able to say something like:

maven_jar(
    name = "guice-library-no-aop",
    artifact = "com.google.inject:guice-no-aop:4.2.2",
    sha1 = "<sha1>",
)

Consider:

  $ wget https://repo1.maven.org/maven2/com/google/inject/guice/4.2.2/guice-4.2.2-no_aop.jar
  2019-11-08 17:41:09 (7.44 MB/s) - ‘guice-4.2.2-no_aop.jar’ saved [520748/520748]
  $ unzip -t guice-4.2.2-no_aop.jar | grep -i "com/google/inject/internal/asm" | wc -l
  0

which is good, nothing is shaded. All other Guice distributions in the wild would produce this:

  $ wget https://repo1.maven.org/maven2/com/google/inject/guice/4.2.2/guice-4.2.2.jar
  2019-11-08 17:46:09 (10.8 MB/s) - ‘guice-4.2.2.jar’ saved [846627/846627]
  $ unzip -t guice-4.2.2.jar | grep -i "com/google/inject/internal/asm" | wc -l
  32

[GedMarc]

You would need ASM and CGLib, the bytebuddy has been on the go for quite a long time now as well,

In order to support JDK 13 fully however, to build JLink and the such, you also need a valid module declaration. Things did change in 12 and 13 a bit further down the line as well.

Hope you come right though!,

[GedMarc]

Oh whats it is worth, i that found bytebuddy itself is running an ASM that doesn't support JDK 13 currently (2019/11/08),
The JDK has an internal ASM, so ASM should always be released before the JDK

It would be great for hibernate, guice, bytebuddy, jdk 9 and up, everything to split asm out.

One of things in modular world, is the hard limit by ASM on method generation to 64kb. If the collective module declarations exceeds this java dies. xD

[davido]

@mcculls

My CL to switch using guice-no-aop artifact in Gerrit Code Review was rejected with the following comment:

Guice's public documentation mentions: [1]

"This version also lacks fast reflection and line numbers in error messages.
For this reason, we recommend Guice+AOP even in applications that don't
use method interceptors."

To me, this sounds as if we shouldn't use Guice without the AOP feature
even if we don't use the feature itself.

[1] https://github.com/google/guice/wiki/OptionalAOP

[mcculls]

That advice is not saying don't use the no-AOP build, just that there can be some benefits of using the AOP build even when you don't need AOP. These benefits will depend on your application.

Wrt. fast reflection... method invocation via reflection in old JDKs could be slow as it involved JNI calls, which was why generating bytecode to do the same could turn out to be faster (depending how often the call was made, etc.) Modern JDKs will now do the same as "fast reflection" and generate Java code for reflective operations once they go above a certain threshold, controlled by the sun.reflect.inflationThreshold system property. So it's not necessarily true that the no-AOP build will be slower - it depends on the application. You'd need to compare the two to see if it made a noticeable difference.

Wrt. line-numbers, again it depends on your use. Assuming your application doesn't regularly throw error messages involving injection then the no-AOP build won't make any difference because it's not throwing errors to begin with. You'd only see a difference if the error involved a missing binding and you needed the additional line number information (in the worst case you'd still have a nearby location from the standard stack trace which you could work along from.)

You could always make it configurable, so the person deploying could choose between the two flavours. That way if you really needed the extra information when debugging an error you could temporarily swap in the AOP version.

Finally as a data point: the no-AOP build is used in Apache Maven because they don't need method interception and because it works better with a wide range of JDKs. The lack of "fast reflection" isn't a noticeable issue as reflective method calls (to call setters) don't happen often enough to warrant bytecode generation - each particular setter is only called a handful of times during a build. Likewise (so far) they haven't needed the additional line numbers to debug binding issues.

[GedMarc]

Agreed,
You can get a guice built in JDK13/14 (part of guicedee) with CGLIB and AOP

Looks like 1.0.1.3 isn't on the site yet taking forever for that mvnrepository site to update now

You can switch -jre13 for -jre8, or 12. the default is on JRE11

https://mvnrepository.com/artifact/com.guicedee.services/guice

https://search.maven.org/artifact/com.guicedee.services/guice/1.0.1.3-jre13/jar

All extensions under com.guicedee.services.extensions

[davido]

Release 4.2.3 was conducted that is JDK 13 compatible.