Skip to content

Once guava-jre requires Java *9*, investigate merging the 2 "flavors" into a multi-release jar #5621

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

Closed
cpovirk opened this issue Jun 22, 2021 · 8 comments
Labels
P4 no SLO package=general type=other Miscellaneous activities not covered by other type= labels

Comments

@cpovirk
Copy link
Member

cpovirk commented Jun 22, 2021

Someday in the distant future, guava-jre will require Java 9.

(My assumption has usually been that we will skip requiring 9 and wait until we have enough reason to require some even higher version. If so, we can wait for that. But the issue I'm now filing will give us at least a small motivation to pull the trigger on Java 9 rather than wait. Then again, our hand is often forced by the people who depend on us, who may likewise by inclined to wait for a version higher than 9....)

When that happens, we could consider trying to eliminate the split between guava-android and guava-jre by putting both into a single multi-release jar. This would be workable only if:

  • Android tools reliably look at only the classes in the "root" section of the jar. [edit: This seems unlikely, given that Android builds use (among many other things) javac.]
  • Non-Android tools reliably look at the classes under META-INF/versions/9.
  • Users can deal with the doubled jar size.
  • The spec for multi-release jars is evolved to eliminate the requirement that "Every version of the library should offer the same API." (For all I know, this may have happened already. But I suspect not.)
  • Probably various other things happen. But I'm already feeling that this is a pipe dream :)

[edit: I have had some trouble with multi-release jars, as discussed in jspecify/jspecify#245. If we're lucky, it will turn out that part of the problem is that I was doing something wrong. But it does sound like there may be legitimate tool limitations at this point. Maybe those will be fixed before we even investigate multi-release jars for Guava.]

@cpovirk cpovirk added type=other Miscellaneous activities not covered by other type= labels package=general P4 no SLO labels Jun 22, 2021
@shreelakshmijoshi
Copy link

Hi @cpovirk !
I am a new open source contributor and would like to know about this issue, could you help me understand the issue by providing some extra references ?

@cpovirk
Copy link
Member Author

cpovirk commented May 10, 2022

I'm going to close this, since the requirement that "Every version of the library should offer the same API" still exists—and is essentially "enforced," intentionally or not, by at least some tools. If something changes, we can reopen this.

@cpovirk
Copy link
Member Author

cpovirk commented May 23, 2024

If we get far enough with #6567, then the guava-jre and guava-android APIs someday might match after all. That might make this a possibility—again, subject to the caveats in my original post. [edit: But it also means that there's relatively little to be gained by undoing the split, since picking the wrong flavor has fewer ill effects (though not zero).]

One significant concern would be that we'd be committing to keep the APIs in sync again—or to re-split guava-jre and guava-android in the future. A re-split would likely end up more disruptive than just keeping the split all along :( I don't know how likely it is that we'd want to re-split, since I don't know what the future holds for library and language desugaring.

Another thing we'd have to think about, though, is whether the Gradle Module Metadata could be made to understand that the 2 flavors of Guava have been collapsed down to 1.

My guess is that we still won't do this. But I wanted to leave the notes while they were on my mind.

@cpovirk
Copy link
Member Author

cpovirk commented May 28, 2024

(I keep thinking "Oh, but what if we kept the APIs exactly the same but put @DoNotCall+@Deprecated on any that people shouldn't use from Android?" But then I keep remembering that that doesn't work because new javac versions will look at the guava-jre APIs even when targeting guava-android (barring --release 9, which is the wrong thing to do when targeting Android). We could in principle still attempt something roughly along those lines, but it would have to be a custom Error Prone checker (which helps only Error Prone users) that somehow determines whether we're targeting Android or not. Or we could maybe just add the APIs and hope for the best. That approach might work best if the APIs were separated out into distinct classes, like if we offered a Records or MoreGatherers class. Really, such separate classes might be something we could consider even if Guava keeps targeting Java 8. But I wouldn't be surprised if that turned out to confuse some build tools.)

@cpovirk
Copy link
Member Author

cpovirk commented Sep 30, 2024

(I also periodically think things like "If we had a multi-release jar, then we could refer to Record from the Java 14+ version." [edit: And here I'm speaking only about in implementations, not in API, since API has to match across versions.] However, while I am not totally sure which version Android would use at runtime, I'm confident that it won't use the Java 14+ version in exactly the circumstances that we want: It will pick (I assume) one or the other, without a way to know which version of Android the app will be run under. That means either that it will lack Record support when we might want it (even before we take into account that arbitrary versions might want it because of desugaring!) or it will attempt to run the Record-support code when it doesn't work. As a result, we'd end up needing to guard the checks no matter what. Maybe this will stick with me now that I've written google/gson#2744 (comment).)

@cpovirk
Copy link
Member Author

cpovirk commented Feb 7, 2025

This issue wasn't meant as a more general issue about multi-release jars, but I just started linking to it as one, so I'm going to post another general thought about multi-release jars :)

I wonder if multi-release jars could reduce the need for Animal Sniffer suppressions. Suppressions don't always work as we'd like. Granted, I'm not sure if the problems we've seen (such as with MethodHandle, with lambdas, and with synthetic methods) are likely to come up with Java 9+ APIs: Aside from MethodHandle itself (for which we don't need multi-release jars, at least one the JRE), the problems arose mostly with classes related to Stream, I think? I'm sure that some of them still could come up with Java 9+ APIs (including a problem with nested classes, I think?), but we just haven't been using them even conditionally very often at all, let alone in cases in which we need suppressions.

@cpovirk
Copy link
Member Author

cpovirk commented Mar 24, 2025

More notes on multi-release jars more broadly:

  • @rovarga observes that we could build the exact same source files twice for guava-jre: once at the minimum level that we support (currently Java 8) and once at a level that allows for improved class-file features (perhaps Java 11 to pick up both indy string concatenation and nestmates, especially given that essentially no one is going to try to run under Java 9 or Java 10). But that of course doubles the jar size. In theory, we could do better by including a second copy only for files where those features would make a significant difference, but that would require us to know which ones they are :)
  • [edit: rovarga also mentions the multi-release jar angle for SequencedCollection support.]
  • I have been playing around with the idea of providing our own implementation of Stream for certain cases. This is usually a bad idea for a variety of reasons, including that Stream is an interface to which methods are often added. A multi-release jar might be the easiest way to do this if we really do want to do it. (Such a jar could contain a version of our class for JDK <current version + 1> that would fall back to using the normal Stream, since that JDK might turn out to add more methods that we haven't implemented. [edit: Oh, wait, the new methods in Stream are in fact default methods, as you'd presumably expect. So we don't need to fall back to the JDK's Stream implementation for correctness. Still, we may wish to fall back for performance reasons (though the default implementation of, e.g., gather looks maybe not necessarily terrible?), but there's an obvious tradeoff if our implementation is faster in some cases...])
    • If a multi-release jar doesn't go well, we have other options, from generating the implementation class dynamically to providing OurStream, OurStream9, OurStream16, etc. (with code to reflectively choose among them at runtime). The former in particular has some chance of helping even if the JDK adds methods to Stream in the future, but I'm not confident that we could predict how to generate the correct implementation for all such future methods today.
  • For notes on Android and multi-release jars, see https://issuetracker.google.com/issues/134377851.

@cpovirk
Copy link
Member Author

cpovirk commented Apr 2, 2025

I've previously said somewhere or other that multi-release jars might not be as good as reflection from the perspective of our Android users. I should note, though, that we need to take care when using reflection: As came up in issue 134377851, and as I noted in #7759, we need to be careful to check for exactly what we want to use [edit: or, better still, a proper check on the Android runtime version, since an API could theoretically be present but non-functional or at least buggy, as I think Charles Munger has warned us about some API in the past?], not just a single sign that we're on (say) Java 9: Under some versions of Android, it turns out that VarHandle is visible to reflection but findVarHandle is not present. [edit: But VarHandle is kind of a bad example because it's not usable even when it's present, at least so far.]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 no SLO package=general type=other Miscellaneous activities not covered by other type= labels
Projects
None yet
Development

No branches or pull requests

2 participants