Figure out when to require Java 11 for new Guava versions

[cpovirk]

Probably not soon :) I just had another note I wanted to make about this, so now seems like the time to start collecting those notes.

• Since we're a dependency of so many other libraries (including ecosystems like Google Cloud), we aren't likely to drop support for Java 8 until such libraries do. Hopefully they won't take too long, given the relatively small amount of support provided for Java 8 nowadays from libraries and JDK vendors.
• Merely requiring a newer javac to build Guava could bring us some of the advantages of this without most of the downsides.
• But it would be even more convenient not to have to put Java 9+ features behind runtime checks of any kind.
  • [edit: also for VarHandle and for ProcessHandle in From Guava 32.0.1-jre, Files.createTempDir() and FileBackedOutputStream can fail or create un-deletable directories/files when used from a Windows service #6634]
• And it might eventually be nice to be able to put Java 9+ APIs (or other language features) in our public API. (We can't do that today, even with multi-release jars [edit: well, at least not without isolating any such new APIs to separate classes].)

[cpovirk]

I think I failed to actually mention the new "note I wanted to make about this," which was:

• JDK 8 has a bug in its reflection implementation, which we'd trigger if we omitted our checker-qual dependency. Granted, we can always decide to omit that dependency anyway, hurting JDK8 users who perform reflection on Guava types (and don't make checker-qual available themselves) but not hurting typical JDK8 users. That's in contrast to dropping Java 8 support, which of course would hurt all JDK8 users :)

[cpovirk]

It turns out that the latest release of jsinterop-annotations is built to require Java 11. That said:

• We somehow don't declare a dep on that library, even though we use things from that package in guava-gwt. Are we relying on users to provide it themselves? (This is another case in which it would be nice for our external GWT tests to be as thorough as our internal ones: b/169936479.) Ah, might the GWT plugin provide it automatically? Are the effects any different on J2CL than on GWT? (Perhaps J2CL users are even more likely to provide the dependency themselves?)
• I don't know how much the annotations artifact changes over time. I guess @JsNonNull was new in 2.0 or so, but I haven't looked at what changes in 2.1.0. Maybe we can get away with not updating for a while.
• Even if we eventually need to require Java 11 for this reason, we might be able to require it only for guava-gwt.
• Maybe things actually kinda sorta work if only annotations are built for Java 11, given how Java is usually tolerant of missing annotations?
• Oh, and this is all just in sources seen only by the GWT and J2CL compilers. Maybe we should just increase the -target for our GWT build? Unfortunately, I think that might affect both client (JavaScript) and server (bytecode), which would mean that users do need to upgrade. Hmm, I thought guava-gwt contained actual compiled classes (not just sources to be compiled by the GWT/J2CL compiler later), but maybe that is only for GWT-RPC, which we don't support anymore. If we do still need to provide server sources, perhaps it's possible to compile them with a different -target than the others.
• Or I guess we don't even need to change our -target: We just need to use a compiler that would recognize the newer bytecode (assuming that everything works OK for users at runtime). That's more like Require Java 11+ to *build* Guava but continue to support Java 8 at runtime #6549. I'll note this there.

[cpovirk]

Java 11 brought nestmates, which would let us make fields like this one private without any performance hit from the need for bridge methods. That hit is likely to be unnoticeable for most methods, small even for CharMatcher, and avoidable by assigning the result of the CharMatcher accessor call to a field (rather than calling the accessor over and over). Still, it would be nice for a single innocent-looking keyword not to have the potential to affect performance. And making things private is nice. (It's even required for similar constructs in Kotlin.)

Admittedly, making nested classes' members private is less important when the class that the API is part of is final, since that makes clear that any package-private methods aren't being called more widely through a subtype. And there are additional arguments for omitting private: One is that private doesn't further restrict visibility, so we'd rather write leave it out for brevity. Another (somewhat contradictory) argument is that the lack of private hints that the API is being used from the nested class's enclosing class (though of course Java would permit the call even with private, just with the possible performance hit discussed above).

Anyway, the point here is that we'd pick up this improvement (and others, like better string concatenation) if we didn't have to target Java 8. (Hmm, I guess we could theoretically provide a multi-release jar with the exact same classes built for both versions, just with different -source -target flags! This would even provide yet another partial defense against problems like that from #3990. But that would double the size of Guava (and possibly upset tools that get confused by mrjars) for very little upside.)

(I was tempted to add that we could consider targeting a newer version of Java for guava-android, since newer bytecode gets converted to Android bytecode that supports even old versions of Android. But of course we say that guava-android is usable under a JVM, too, so that wouldn't work.)

[cpovirk]

Other potentially nice things for the future:

• @Deprecated(forRemoval = true)
• the ability to drop a boatload of code in Throwables (including references to ThreadDeath, which is (speaking of which!) deprecated for removal), in ChecksumhashFunction, in Hashing, and perhaps elsewhere
  • [edit: I think we could actually remove a bunch of that code even now, since we already build with newer versions of javac.]
• we hope someday nullness markers
• var, pattern-matching instanceof (Java 16+), records (Java 16+, maybe even in our API somewhere), and probably other nice things
• [edit: Runtime.version, mostly for use in our tests, though we primarily check isJava8(), so any checks based on Java 8 would just go away entirely :)]
• [edit: System.Logger has come up before]
• [edit: SequencedCollection, Java 21+]
• [edit: Maybe hypothetically someday JDK-8072840 for pre-sizing for our Collector implementations. (hat tip to fmeum)]
• [edit: Omitting enclosing-instance fields from inner classes that don't use them, Java 18+]

(As always, we'd want to keep in mind that we also still target Android, including fairly old versions, so we might not want to make at least some changes until we can make them there, too.)

[cpovirk]

Java 11 brought nestmates, which would let us make fields like this one private without any performance hit from the need for bridge methods.... And making things private is nice. (It's even required for similar constructs in Kotlin.)

I learned yesterday that nestmates also enable runtime access through MethodHandles.Lookup, which would be helpful for the forthcoming usage of VarHandle in AbstractFuture (and, I suspect, would let us simplify its existing usage of AtomicReferenceFieldUpdater) in #7555. We have an easy workaround, but it would have been nice to not have had to fiddle around with the code and then retrace my steps after I forgot my earlier partial understanding and so had to build up a fuller understanding later.

[cpovirk]

More notes:

Historically, one reason that we've maintained support for Java 8 is the Google Cloud Client Libraries. The documentation for those libraries currently says:

Java 8 will continue to be supported by Cloud Client Libraries for Java until at least September 2026.

I'm not sure how much we've pushed on the idea that those libraries could stick with an older version of Guava if Guava were to drop support for Java 8 before Cloud did. I would expect that approach to work well in practice for Cloud users, but I could imagine that some users would have reservations about running with a newer version of Guava than the one that Cloud recommends, and that may be enough to stop the idea.

Anyway, the reason that I'm thinking about this yet again is that the majority of my week is going to reworking our nullness annotations to work better with Java 8 (e.g., #7566). That's turned what should have been a straightforward update of https://github.com/google/guava/tree/jspecify-preview into a Project.

[cpovirk]

Whenever we do do this, Cloud (and others) may want us to bump our major version. It's left a bit up in the air in https://opensource.google/documentation/policies/library-breaking-change. Additionally, that policy refers to "vendor support," which can mean a lot of things in the context of Java (and something else in the context of Cloud). It's something for us to look into.

[kevinb9n]

I see a variety of "implementationy" advantages, but the bigger issue is that Guava public APIs are unable to reference any new library types, so they're unable to provide the best level of service to clients who are on the newer versions. SequencedCollection is a fine enough example of this.

I suspect that mr-jars aren't going to be an easy way out of this. It would be useful if we could push on "okay, what does happen if Guava 34.0 requires Java 21 (or even 17, or even 11)?"

There has to be a solution that involves certain users and certain ecosystems simply getting pinned to Guava 33 for a while and living with the pain of that. Otherwise, how would it ever be possible?

Does anyone at G feel like fleshing out that picture?

[cpovirk]

My feeling has been that the API needs just haven't been that big. SequencedCollection is fine but hasn't felt compelling on its own. Nullness markers would of course get me to reconsider very quickly, despite the downsides to users stuck on older versions :)

For pure additions, we also have the option to put whatever we want in another class: There's nothing to stop us from adding com.google.common.base.Records today with getCanonicalConstructor or whatever; what would get us in trouble is "only" additions/changes to existing classes.

I hope to have a better picture of what users want as Google and other Guava users continue to get experience with Java 21 and higher.

[JanecekPetr]

Is there ever a world where Guava updates the required Java, and changes the artifact coordinates + the package names together with that to allow people migrating from one to the other explicitly, perhaps with an intermediary interval in between where applications could have both on their classpath? Or do we consider Guava to be so widespread that such a migration is unlikely to ever happen? Jackson, for one, is preparing to do such a release.

With the current Guava being ... stable, can you imagine a world with a new major release where the old branch would still get occasional security updates, but the development would shift to the new world? Quite a lot could potentiall be dropped at that point, and even more could be possibly added.

I imagine such a project would have to be business-driven, though, and that sounds unlikely :)

[rovarga]

LoOoOoL, were it not for the size increase, I would not have taken a closer look 😆

Picking a single class at random:

nite@nitebug : ~/x$ unzip ~/.m2/repository/com/google/guava/guava/33.4.5-jre/guava-33.4.5-jre.jar
[...]
nite@nitebug : ~/x$ ls -l ./com/google/common/util/concurrent/TimeoutFuture.class META-INF/versions/9/com/google/common/util/concurrent/TimeoutFuture.class
-rw-r--r--. 1 nite nite 5259 Feb  1  1980 ./com/google/common/util/concurrent/TimeoutFuture.class
-rw-r--r--. 1 nite nite 5529 Feb  1  1980 META-INF/versions/9/com/google/common/util/concurrent/TimeoutFuture.class

I thought it was a packaging problem, but then:

nite@nitebug : ~/x$ md5sum ./com/google/common/util/concurrent/TimeoutFuture.class META-INF/versions/9/com/google/common/util/concurrent/TimeoutFuture.class 
480f0d2ec77600d7fabfb6bb8ab67747  ./com/google/common/util/concurrent/TimeoutFuture.class
b69653a81f320c395dbb95bb5112654d  META-INF/versions/9/com/google/common/util/concurrent/TimeoutFuture.class

showed differences which with javap -v, ignoring clutter, showed something worthwhile:

+BootstrapMethods:
+  0: #144 REF_invokeStatic java/lang/invoke/StringConcatFactory.makeConcatWithConstants:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
+    Method arguments:
+      #140 inputFuture=[\u0001]
+  1: #144 REF_invokeStatic java/lang/invoke/StringConcatFactory.makeConcatWithConstants:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/invoke/CallSite;
+    Method arguments:
+      #142 \u0001, remaining delay=[\u0001 ms]

i.e. taking advantage of JDK9+ string concatenation, so I thought this was intentional 😄

So yeah, I have 33.4.4 slated for next round of upgrades just because it gets rid of JSR305, but was holding back the upgrade to 33.4.5 to mull over the cost/benefit part of it. #7732 has the potential to being an issue we'd hit right after that.

Overall though, I think this goes to reinforce my point above: providing a -jre11 version would provide real benefits to users without any code changes 😄

[rovarga]

A data point: the packaging in 33.4.5 is equivalent, but JDK9+ classes are ~.44% larger:

nite@nitebug : ~/x$ find com/ -type f | wc -l
1968
nite@nitebug : ~/x$ find META-INF/versions/9/com/ -type f | wc -l
1968
nite@nitebug : ~/x$ du -s com/ META-INF/versions/9/com/
10888   com/
10936   META-INF/versions/9/com/

[rovarga]

Thinking about this issue a bit more: let's assume we have -android, -jre for Java 8+, -jre11 for Java 11+ and -jre21 for Java 21+. I think maven et al. would consider -android <= -jre <= jre11 <= --jre21.

IFF that is the case, it should not be a API compatibility problem for jreXX to mix in Java XX interfaces into existing contracts -- i.e. ImmutableCollection could be a SequencedCollection in -jre21 IFF the Guava versioning contract were to preclude downstream from switching flavors. That is to say: going from 33.4.6-jre21 to 33.4.7-jre11 is an incompatible change, contrary to what SemVer would lead you to believe.

With that assumption, it would be relatively easy (yeah, it is not me who does that! 😄) to provide both forward- and backward-compatibility by:

1. providing SequencedCollection methods in < -jre21 artifacts
2. providing ImmutableCollection implements SequencedCollection in >= -jre21 artifacts (and perhaps via META-INF/versions/21, but that opens a can of worms)

I think the forward-compatibility part provided by 1., i.e. an efficient SingletonImmutableList,getFirst(), is more important. The second part would be provided by users explicitly linking to -jre21 -- but that may to be a headache to final packager, who needs to be cognizant that '33.4.7-jre11is an API-incompatible downgrade from33.4.6-jre21`. In my experience this is an issue each packager would encounter at most once and once fixed, they'd be happy as a clam.

[rovarga]

I would like to point out that while the discussion so far has revolved around

• Java 11 because of all the things entailed in Java 9+ world and nest-mates
• Java 21 because of SequencedCollection

we should also consider Java 17 being a major milestore due to sealed classes, which renders quite a bit of code around ImmutableCollection enforcing invariants superfluous (IIRC).

[cpovirk]

(lots more to follow up on here, but as for fixing #6614 (comment)...)

I've been trying to understand how our exclusion for the extra files isn't working. And I think I've figured it out: I don't think we're actually using maven-jar-plugin (at least for the main Guava code; maybe it's still used for testlib or tests). Instead, we're using maven-bundle-plugin for OSGi reasons.

I'm not immediately seeing a way to exclude files in its configuration. Maybe we could instead delete the files entirely (which, for all I know, might also lead to more accurate testing or something, should some tests try to use the directory of class files instead of the jar (somewhat unlikely for Guava, whose tests live in a separate Maven module)??). Or we could modify the bundle afterward. Or maybe we could produce the OSGi configuration ourselves in some other way.

Or we could try something else I've speculated about for performance reasons: We could try to compile fewer files during the second (i.e., modular / Java 9). We might pick a standalone file from each package (e.g., NullnessCasts.java for most packages).

And actually, to my surprise, it looks like we may be able to restrict the compilation to only module-info.java? I thought for sure that we'd have to have at least one class from each directory.

So I think I have a solution.

[cpovirk]

Coming back to the other stuff:

• While I'm not happy to have introduced problems for people (sorry again!), I'm at least happy that this helps justify the weird series of releases that we put out to let people upgrade only partially :) I also already had to bump one of our own projects only to 33.4.3 for the moment....

[cpovirk]

Sorry, I didn't meant

[cpovirk]

...didn't mean to publish my comment or close this, sorry....

More to come shortly.

[cpovirk]

• Intentionally using multi-release jars for string-concat/nestmates reasons does, as you point out, make perfect sense, with the only downside I know of being the obvious one that you've also pointed out. I left some notes about that in Once guava-jre requires Java *9*, investigate merging the 2 "flavors" into a multi-release jar #5621 (comment).
• An additional "version" of Guava (beyond the existing -jre and -android "versions") would be another option. That said, I have single-handedly caused enough trouble with clever versioning (with those existing versions and with listenablefuture) that I would prefer to just raise the baseline if possible. Still, I should not dismiss this out of hand, and I'll try to remember to come back to it when the time comes.
• I agree that we should add the SequencedCollection methods to Guava. It's possible that we could do more. See also Support JDK 21 Sequenced Collections #6903. For annoying technical reasons, even adding the methods may be harder to do for reversed() than it should be, but luckily that method largely duplicates the existing ImmutableCollection.reverse() for us.
• I'd be interested in what you have in mind for ImmutableCollection invariants. It's not something I've given much thought to, and we still have a while before it's an option, but if you can quickly handwave about what you have in mind, it's another thing that I'll try to keep in mind.
• Also, I'll add something to the release notes for 33.4.5 about the accidental size increase.
• Let me know if I failed to respond to something.
• On to jpms: add 'require static' entries for annotation processing #7732!

[rovarga]

(lots more to follow up on here, but as for fixing #6614 (comment)...)

I've been trying to understand how our exclusion for the extra files isn't working. And I think I've figured it out: I don't think we're actually using maven-jar-plugin (at least for the main Guava code; maybe it's still used for testlib or tests). Instead, we're using maven-bundle-plugin for OSGi reasons.

ah, so I would suggest taking a look at bnd-maven-plugin. It needs a minor workaround to integrate with m-j-p, but after that it should be breeze to get running.

[cpovirk]

Nice, thanks. It looks like #7741 worked OK, so I'll probably stick with that for now. But if we have more problems, I'll try to remember to look at moving over to bnd-maven-plugin.