Thanks for looking into this, Stuart! Some replies inline.. On Tue, Jul 5, 2011 at 3:20 PM, Stuart McCulloch <[email protected]> wrote:
> Hi folks, > > Background: > > The Guice project tree used to include a sub-set of Guava source code, > manually refactored under "core/src/com/google/inject/internal/util". Guava > is an internal implementation detail, just like CGLIB and ASM, which is why > it is embedded under an internal package inside Guice. Recently this code > was removed and replaced with a dependency to guava-r09.jar whose classes > are now refactored and shrunk by jarjar at build time. Unfortunately, > because of a limitation in jarjar (see an earlier discussion under > http://code.google.com/p/google-guice/issues/detail?id=264) it has trouble > shrinking Guava down to the subset of classes which are strictly required by > Guice. This means that the current Guice jar built from trunk is now just > over 1Mb compared to just under 700Kb for Guice 3, which might not be an > issue for most desktop developers but could be a concern for mobile > developers. > > Enter ProGuard: > > There are several jar shrinking tools around, one of the better ones being > ProGuard http://proguard.sourceforge.net/ However it doesn't appear to > have the capability to preserve the classname when you ask it to refactor > packages, instead it renames them to short names such as A, B, etc. which > isn't good for debugging. The best solution seems to be a combination of > ProGuard (for shrinking) and jarjar (for package refactoring) and with a bit > of tweaking I can get the Guice jar down to around 830kb (still not as good > as when we manually copied in the subset of Guava source, but not bad). > I haven't messed around with ProGuard much, but looking at the reference page @ http://proguard.sourceforge.net/index.html#/manual/refcard.html shows a few options that seem like they'd get the job done for not renaming: --keepnames (and a few others with that prefix), --dontoptimize, --dontobfuscate, --keeppackagenames, --keepparameternames. > > Fragility Concerns: > > While adding ProGuard to my prototype Guice build I ran into a couple of > cases where extensions and tests relied on Guava methods that weren't used > in Guice core. Because ProGuard (and jarjar) operate on Guice core before > the tests or extensions are built, it has no knowledge that these methods > should be retained, which leads to odd test failures. While I can add > explicit instructions to tell ProGuard to keep these methods and classes > around, I'm concerned that this makes the build much more fragile. An > extension developer could easily depend on a new method in Guava (since it's > on the project classpath) and this new method wouldn't be repackaged inside > Guice unless the ProGuard instructions were updated (or unless Guice core > just happened to also use the same method). > > Note this wasn't an issue when we manually included the sub-set of Guava > source code, because the extensions built against the classes under > "core/src/com/google/inject/internal/util" and not against the whole of > Guava. > > Possible Options? > > 1) Do nothing (and live with the larger jar size where we basically embed > all of Guava) > If we need to, I think this is a valid choice. The jar for mobile folks will be smaller anyway, since it won't include cglib or asm. Also, the only bits of Guava that are kept are any referenced (transitively) classfiles. In practice, that ends up as most of com.google.common.base & com.google.common.collect, plus a few scattered com.google.common.ioclasses. For the non-mobile version, it's about a ~400k increase the main jar size. > > 2) Add ProGuard to the core build (and live with having to update the > instructions when extensions use new Guava methods) > IMO, this is the best option. The extensions aren't updated all that often, and it would be rare that one needs to use a method/class that the core isn't using. An alternative here would be to make the extensions ProGuard/jarjar their own Guavas. That would slightly increase the size of extensions that use them, but not by much. > > 3) Revert to including the sub-set of Guava source code (harder to update, > but perhaps more manageable / less fragile?) > Definitely -1 here. Pointing to Guava gives us a lot of nice things, such as making it a whole lot easier to update when new Guava releases come out (such as the soon-to-be-shipped r10 which removes FinalizableReferenceQueue entirely, fixing issue 288 for us just by dropping in a new jar). > > 4) Build Guice core + extensions without any jarjar'ing or ProGuard'ing, > only do that as a final distribution / packaging step > This sounds like an an implementation detail and we'd still have to resolve #2 or #3... and I'd very much like to avoid people accidentally building against the wrong version. > > 5) Make Guava an external dependency, since a lot of people already use > Guava (leads to potential versioning issues) > I'm -0.5 on this, because I don't really want to be tied to Guava versions. Depending on it directly is kind of like making it part of the public API, and I'd rather not do that. sam > > Note option 4) would be moving more towards the model where everything is > built as simple Java projects and we only apply jarjar, etc. to get a > "nodeps" binary (to borrow the phrase from CGLIB) at the end. The downside > of this approach is that the unit tests would run against the un-jarjar'd > binaries, but there's no reason why we couldn't do an extra round of tests > against the final jarjar'd artifact. The other worry is that people might > take the interim jars which haven't been jarjar'd by mistake (but then again > there have been requests in the past from users to produce such artifacts, > see http://code.google.com/p/google-guice/issues/detail?id=289). > > Would like to hear people's thoughts on these options, since I don't think > there's an obvious "correct" choice* > > -- > Cheers, Stuart > > (* of course if everyone used OSGi then we wouldn't need to worry about > embedding and could just declare we needed the appropriate versions of > ASM/CGLIB/Guava ;) > > -- > You received this message because you are subscribed to the Google Groups > "google-guice" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/google-guice?hl=en. > > -- You received this message because you are subscribed to the Google Groups "google-guice" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/google-guice?hl=en.
