[wildfly-dev] private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...
Gunnar Morling
gunnar at hibernate.org
Sun Feb 21 13:24:23 EST 2016
I've updated the implementation based on ByteBuddy at
https://github.com/gunnarmorling/hibernate-orm/tree/HHH-10536 so it's
passing all tests in ORM now. The implementation is not polished but
should be good enough for doing some testing on WF.
One nice side-effect of using ByteBuddy is that was trivial to
implement BasicProxyFactory so it generates types that use actual
typed fields as the basis for the property getters/setters instead of
the hashmap implementation used in the Javassist implementation. So
that piece should be a bit more efficient, too.
Let me know if there is interest in this implementation, I'd then
clean up some things and also could take a look into providing a
ReflectionOptimizer implementation.
> So I can use Javassist or ByteBuddy or ASM or ... to deal with (1) as long as that choice does not leak into (2). That is the important part to me.
+1.
As said, the same could be done using plain ASM, but ByteBuddy makes
it just much simpler to achieve this goal.
--Gunnar
2016-02-19 21:59 GMT+01:00 Steve Ebersole <steve at hibernate.org>:
> Personally I do not see this distinction as the important factor. I get
> what you are saying, and if we intentionally were exposing Javassist as an
> API detail I would agree that copying and shading would be bad. But I say
> it is not important because even though I do *not* view Javassist as
> something we intentionally expose as an API detail I still think it is
> important to copy/shade only as a last resort.
>
> The mistake in our current OOTB bytecode manipulation implementation seems
> to be that we expose some of its impl details (usage of Javassist) into the
> manipulated bytecode. What I like specifically about the some of the
> suggestions here so far (and Gunnar's work IIUC) is that it splits this
> problem into distinct concerns:
>
> Actually performing the manipulation and/or proxy generation
> Runtime usage of that manipulated class or proxy.
>
> So I can use Javassist or ByteBuddy or ASM or ... to deal with (1) as long
> as that choice does not leak into (2). That is the important part to me.
>
> That's the best option IMO if possible. And I think it is.
>
> On Fri, Feb 19, 2016 at 1:56 PM Paul Benedict <pbenedict at apache.org> wrote:
>>
>> I think the answer should depend on how the Hibernate developers view
>> Javassist. Is Javassist seen as a pluggable part of the API? Or is it seen
>> as fundamentally an internal component? If it's the former, then do not
>> shade it; otherwise do shade it. I think the latter is very sensible as long
>> as that perspective is maintained in the code base.
>>
>> Cheers,
>> Paul
>>
>> On Fri, Feb 19, 2016 at 11:47 AM, Scott Marlow <smarlow at redhat.com> wrote:
>>>
>>>
>>>
>>> On 02/18/2016 01:17 PM, Carlo de Wolf wrote:
>>> > Looks to me like Hibernate is exposing bad proxies to the user.
>>>
>>> Its not bad or new, just how we always did it. Other Javassist users
>>> have also done the same and ended up shading Javassist classes.
>>>
>>> >
>>> > Would it not be possible to use a custom class loader at the point
>>> > where
>>> > the proxy is defined?
>>> > Than you could use one that takes the version of javassist that
>>> > Hibernate requires and delegates everything else to the deployment
>>> > class
>>> > loader.
>>>
>>> This sounds similar to my https://github.com/wildfly/wildfly/pull/8474
>>> pull request that changes the Hibernate ORM static module to export the
>>> Javassist classes.
>>>
>>> >
>>> > I don't like to see any sort of shading as this means the full
>>> > maintenance burden of those classes are carried over into Hibernate.
>>> >
>>> > Carlo
>>> >
>>> > On 02/18/2016 12:11 PM, Sanne Grinovero wrote:
>>> >> It seems we're discussing this issue in multiple places,
>>> >> so to let you all know I'll repeat it hare:
>>> >> I think shading is a really really bad idea :)
>>> >>
>>> >> Could we try to have the enhanced entities to not need Javassist in in
>>> >> their *direct* classloader; we can still have a normal Javassist as a
>>> >> module dependency of Hibernate?
>>> >> That would require to just make sure the generated bytecode doesn't
>>> >> directly refer to Javassist types but uses an indirection controlled
>>> >> by Hibernate code.. which in turn can use Javassist or even
>>> >> alternatives in future, if we'd like to experiment.
>>> >>
>>> >> I'm not familiar enough with Javassist to know if that's an option
>>> >> as-is but we can either improve Javassist to allow such a thing or use
>>> >> some alternatives, like Gunnar and Hardy also suggested on the
>>> >> hibernate-dev mailing list.
>>> >>
>>> >> To summarize, I agree with Stuart and would hope that Scott's branch
>>> >> can be improved by minimizing the amount of Javassist code which
>>> >> actually needs to be copied by using some simple delegation to
>>> >> Hibernte types, which in turn can use a private, non-shaded Javassist
>>> >> taking advantage of the isolation provided by JBoss Modules.
>>> >>
>>> >> --Sanne
>>> >>
>>> >>
>>> >>
>>> >> On 12 February 2016 at 03:19, Scott Marlow <smarlow at redhat.com> wrote:
>>> >>> What if Javassist packaged these same (proxy/runtime) classes in a
>>> >>> separate javassist-runtime jar and we shaded only the proxy/runtime
>>> >>> classes? That way we only repackage the same classes that we
>>> >>> included
>>> >>> for this hack test (e.g.
>>> >>> org.hibernate.bytecode.internal.javassist.proxy.*).
>>> >>>
>>> >>> Early testing results of the hack test look good
>>> >>> (https://gist.github.com/scottmarlow/ad878968c5a7c6fbbfb7).
>>> >>>
>>> >>> Scott
>>> >>>
>>> >>> On 02/11/2016 09:04 PM, Stuart Douglas wrote:
>>> >>>> It depends if you are going to shade all the javassist classes or
>>> >>>> just
>>> >>>> the "javassist.util.proxy" package (not sure if this is actually
>>> >>>> possible with the shade plugin).
>>> >>>>
>>> >>>> The main advantage is that you can upgrade javassist to get fixes to
>>> >>>> issues that affect bytecode generation. So if JDK9 comes out with
>>> >>>> new
>>> >>>> bytecodes that the current version of Javassist does not understand
>>> >>>> then
>>> >>>> upgrading javassist will allow the older version of hibernate to
>>> >>>> work
>>> >>>> with classes compiled against the newer JDK version. If all of
>>> >>>> javassist
>>> >>>> is shaded into hibernate then that version of hibernate will never
>>> >>>> work
>>> >>>> with the newer bytecodes.
>>> >>>>
>>> >>>> I think this is less of an issue if you are still publishing the
>>> >>>> non-Javassist shaded hibernate as well as a shaded version, but if
>>> >>>> the
>>> >>>> only published artifact has javassist shaded in then it may limit
>>> >>>> forward compatibility.
>>> >>>>
>>> >>>> Stuart
>>> >>>>
>>> >>>>
>>> >>>> On Fri, 12 Feb 2016 at 12:53 Steve Ebersole <steve at hibernate.org
>>> >>>> <mailto:steve at hibernate.org>> wrote:
>>> >>>>
>>> >>>> Ugh. That is an awful lot of classes copied over. What
>>> >>>> exactly was
>>> >>>> the benefit of this over shading again? I mean both case lose
>>> >>>> the
>>> >>>> ability to simply drop in fixes from upstream Javassist. So
>>> >>>> what
>>> >>>> does this "clone" approach gain versus shadowing?
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> On Thu, Feb 11, 2016 at 7:13 PM Scott Marlow
>>> >>>> <smarlow at redhat.com
>>> >>>> <mailto:smarlow at redhat.com>> wrote:
>>> >>>>
>>> >>>> >>
>>> >>>> >> On 02/11/2016 03:02 PM, Stuart Douglas wrote:
>>> >>>> >> > Have you considered a 3rd alternative, which is
>>> >>>> to
>>> >>>> use a custom
>>> >>>> >> > ProxyFactory instead of javassists built in
>>> >>>> one?
>>> >>>> >> >
>>> >>>> >> > AFAIK the main issue is that javassist proxies
>>> >>>> require access to the
>>> >>>> >> >
>>> >>>> 'javassist.util.proxy.MethodHandler|RuntimeSupport'
>>> >>>> classes. You
>>> >>>> >> could
>>> >>>> >> > create a similar org.hibernate interface, and a
>>> >>>> proxy factory
>>> >>>> >> that uses
>>> >>>> >> > this method handler instead.
>>> >>>> >> >
>>> >>>> >> > Basically you just copy the code from
>>> >>>> javassist.util.proxy into
>>> >>>> >> > hibernate. This is a relatively small amount of
>>> >>>> code, so it
>>> >>>> >> should not
>>> >>>> >> > really add any maintenance burden.
>>> >>>> >>
>>> >>>> >> We talked about this as well via [1]. I
>>> >>>> understand the
>>> >>>> concept but have
>>> >>>> >> not tried doing this. I like this approach as
>>> >>>> well, if
>>> >>>> it works. One
>>> >>>> >> of the cons with cloning that Steve Ebersole
>>> >>>> pointed
>>> >>>> out (see response
>>> >>>> >> on Feb-03-2016 9:01am), is that that users lose
>>> >>>> the
>>> >>>> ability to drop a
>>> >>>> >> different version of Javassist in (since we
>>> >>>> maintain
>>> >>>> our own cloned copy
>>> >>>> >> of the Javassist proxy/runtime code).
>>> >>>> >>
>>> >>>> >>
>>> >>>> >> The proxy code is a relatively small part of javassist,
>>> >>>> so
>>> >>>> unless a bug
>>> >>>> >> is in the proxy code itself this should not be that big
>>> >>>> a deal.
>>> >>>> >
>>> >>>> > Thanks for the encouragement to go down this path. :)
>>> >>>> >
>>> >>>>
>>> >>>> Started a hack attempt at the clone via
>>> >>>>
>>> >>>> https://github.com/scottmarlow/hibernate-orm/tree/javassistproxy.
>>> >>>> Seems
>>> >>>> to pass the Hibernate ORM unit tests.
>>> >>>>
>>> >>>> Scott
>>> >>>>
>>> >>>> _______________________________________________
>>> >>>> wildfly-dev mailing list
>>> >>>> wildfly-dev at lists.jboss.org
>>> >>>> <mailto:wildfly-dev at lists.jboss.org>
>>> >>>>
>>> >>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>> >>>>
>>> >>> _______________________________________________
>>> >>> wildfly-dev mailing list
>>> >>> wildfly-dev at lists.jboss.org
>>> >>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>> >> _______________________________________________
>>> >> wildfly-dev mailing list
>>> >> wildfly-dev at lists.jboss.org
>>> >> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>> >>
>>> >>
>>> >
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> wildfly-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> wildfly-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
More information about the wildfly-dev
mailing list