commit 41887d552a38b5e6b55ca1be6c98b470f090d2e1
author David Ostrovsky <david@ostrovsky.org> Wed Oct 30 08:58:05 2019 +0100
committer David Pursehouse <dpursehouse@digital.ai> Fri Apr 17 15:01:33 2020 +0900
tree c2d42e8fc1392677ccccf26128391fd47cb68855
parent 9262cce382faeea690c1deab4b0feedcb52fd2d3

Switch to using no-AOP guice distribution

Gerrit and transitive dependencies don't use Guice AOP feature. Yet the
default Guice distribution shades cglib and asm libraries to support it.

This complicates the support for newer JDK versions, as the asm and the
Guice library must be upgraded to use newer byte code manipulation
libraries. This is especially a challenge when a new major Java version
is popping up every 6 months.

Given that Guice project already provides distribution without shaded
cglib and asm library: guice-x.y.z-classes.jar and even the one without
AOP feature altogether: guice-x.y.z-no_aop.jar, we can switch to using
the stripped Guice distribution.

Another advantage of using no-AOP Guice distribution, is that it is not
suffering from illegal reflective access warning: [1].

The public Guice's documentation mentions some advantages as for why
the Guice's AOP flavour should be used even in applications that don't
use method interceptors [5]:

"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."

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.

Line numbers: assuming the 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. The only
difference a developer would see if the error involved a missing binding
and developer needed the additional line number information.

We could always make it configurable, so the person deploying could
choose between the two flavours. That way if we really needed the extra
information when debugging an error we could temporarily swap in the AOP
version, by reverting this change.

Moreover, there are also other prominent Guice users who use no-AOP
Guice distribution, like Apache Maven and Sonatype Nexus.

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

Bug: Issue 12448
Change-Id: I4a052850d14d78ce8c298e58fe5e8604f7559049