private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

James Perkins
I don't recall the previous discussion, but is there a reason we couldn't just use a JBoss module with a different slot?

On Thu, Feb 11, 2016 at 8:03 AM, Scott Marlow <[hidden email]> wrote:
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev



--
James R. Perkins
JBoss by Red Hat

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Stuart Douglas


On Fri, 12 Feb 2016 at 06:42 James Perkins <[hidden email]> wrote:
I don't recall the previous discussion, but is there a reason we couldn't just use a JBoss module with a different slot?

Nah, the issue is that these classes need to be referenced by the deployment and hibernate, so whatever version hibernate uses has to be the one exposed to the deployment.

Stuart
 

On Thu, Feb 11, 2016 at 8:03 AM, Scott Marlow <[hidden email]> wrote:
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev



--
James R. Perkins
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

James Perkins
Ah, got it. Makes sense then.

On Thu, Feb 11, 2016 at 11:55 AM, Stuart Douglas <[hidden email]> wrote:


On Fri, 12 Feb 2016 at 06:42 James Perkins <[hidden email]> wrote:
I don't recall the previous discussion, but is there a reason we couldn't just use a JBoss module with a different slot?

Nah, the issue is that these classes need to be referenced by the deployment and hibernate, so whatever version hibernate uses has to be the one exposed to the deployment.

Stuart
 

On Thu, Feb 11, 2016 at 8:03 AM, Scott Marlow <[hidden email]> wrote:
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev



--
James R. Perkins
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev



--
James R. Perkins
JBoss by Red Hat

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Fernando Nasser
In reply to this post by Stuart Douglas
On 2016-02-11 2:55 PM, Stuart Douglas wrote:


On Fri, 12 Feb 2016 at 06:42 James Perkins <[hidden email]> wrote:
I don't recall the previous discussion, but is there a reason we couldn't just use a JBoss module with a different slot?

Nah, the issue is that these classes need to be referenced by the deployment and hibernate, so whatever version hibernate uses has to be the one exposed to the deployment.
Can't we require that that alternative module is specified as a dependency for the hibernate deployments?

It does put the onus on the user but we already end up having to specify a few things for hibernate anyway.

Fernando



Stuart
 

On Thu, Feb 11, 2016 at 8:03 AM, Scott Marlow <[hidden email]> wrote:
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev



--
James R. Perkins
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Stuart Douglas
In reply to this post by Scott Marlow
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.

The inability to change javassist versions could be a major pain for Hibernate later on, as it may mean that older hibernate versions fail to work with newer JDK's if changes are made to the class file format.

Stuart

On Fri, 12 Feb 2016 at 03:03 Scott Marlow <[hidden email]> wrote:
As previously discussed, Hibernate applications need access to the
Javassist runtime classes (see example [1] enhanced application entity
if you didn't know this :).  A proposal was discussed on the
hibernate-dev mailing list that I think is the best short term solution.
  I wanted to raise this issue here also, as I would like to later
create a pull request to bring in a new Hibernate ORM that includes this
change.  So, getting early feedback before we create JIRAs for the work,
is important.

The proposal is to private package (or shade), the Javassist classes, so
that Hibernate ORM has its own copy of the Javassist classes.  On
WildFly, we still would include Javassist for the other components that
use it and for Hibernate applications that have "build-time enhanced
entity classes" by an earlier Hibernate release.

One downside of this change is that Hibernate applications cannot easily
switch to a different version of the Javassist classes.

Another downside is that applications that depend on an older Hibernate
ORM version that includes "build-time enhanced entity classes", will
need to be cracked open, to add dependencies on the Javassist module
(since we will stop automatically adding Javassist to JPA application
deployments).

The advantage of this change, is that Hibernate applications can include
their own version of Javassist.

This will also have an impact on Hibernate build-time enhancing of
entity classes (e.g. enhanced bytecode will no longer depend on the
public Javassist classes).

Scott

[1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
In reply to this post by Fernando Nasser


On 02/11/2016 02:59 PM, Fernando Nasser wrote:

> On 2016-02-11 2:55 PM, Stuart Douglas wrote:
>>
>>
>> On Fri, 12 Feb 2016 at 06:42 James Perkins
>> <<mailto:[hidden email]>[hidden email]> wrote:
>>
>>     I don't recall the previous discussion, but is there a reason we
>>     couldn't just use a JBoss module with a different slot?
>>
>>
>> Nah, the issue is that these classes need to be referenced by the
>> deployment and hibernate, so whatever version hibernate uses has to be
>> the one exposed to the deployment.
> Can't we require that that alternative module is specified as a
> dependency for the hibernate deployments?

The Javassist module is marked as private api, so applications are
discouraged from depending on it.  They certainly can but they get a
warning that the dependency might go away in the future (which means the
app might not deploy in the future).

>
> It does put the onus on the user but we already end up having to specify
> a few things for hibernate anyway.

For native Hibernate api applications, they currently have to add a
dependency on Javassist.  For EE JPA container managed applications, we
automatically add a dependency on Javassist for them.

>
> Fernando
>
>
>>
>> Stuart
>>
>>
>>     On Thu, Feb 11, 2016 at 8:03 AM, Scott Marlow
>>     <<mailto:[hidden email]>[hidden email]> wrote:
>>
>>         As previously discussed, Hibernate applications need access to the
>>         Javassist runtime classes (see example [1] enhanced
>>         application entity
>>         if you didn't know this :).  A proposal was discussed on the
>>         hibernate-dev mailing list that I think is the best short term
>>         solution.
>>           I wanted to raise this issue here also, as I would like to later
>>         create a pull request to bring in a new Hibernate ORM that
>>         includes this
>>         change.  So, getting early feedback before we create JIRAs for
>>         the work,
>>         is important.
>>
>>         The proposal is to private package (or shade), the Javassist
>>         classes, so
>>         that Hibernate ORM has its own copy of the Javassist classes.  On
>>         WildFly, we still would include Javassist for the other
>>         components that
>>         use it and for Hibernate applications that have "build-time
>>         enhanced
>>         entity classes" by an earlier Hibernate release.
>>
>>         One downside of this change is that Hibernate applications
>>         cannot easily
>>         switch to a different version of the Javassist classes.
>>
>>         Another downside is that applications that depend on an older
>>         Hibernate
>>         ORM version that includes "build-time enhanced entity
>>         classes", will
>>         need to be cracked open, to add dependencies on the Javassist
>>         module
>>         (since we will stop automatically adding Javassist to JPA
>>         application
>>         deployments).
>>
>>         The advantage of this change, is that Hibernate applications
>>         can include
>>         their own version of Javassist.
>>
>>         This will also have an impact on Hibernate build-time enhancing of
>>         entity classes (e.g. enhanced bytecode will no longer depend
>>         on the
>>         public Javassist classes).
>>
>>         Scott
>>
>>         [1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
>>         _______________________________________________
>>         wildfly-dev mailing list
>>         [hidden email] <mailto:[hidden email]>
>>         https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>>
>>
>>
>>     --
>>     James R. Perkins
>>     JBoss by Red Hat
>>     _______________________________________________
>>     wildfly-dev mailing list
>>     [hidden email] <mailto:[hidden email]>
>>     https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
In reply to this post by Stuart Douglas

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

If we use a private packaged copy of the jars, in order to use a
different version of Javassist, users would have to get a new version of
Hibernate that is built with that different Javassist (as you point out).

I would like to create a HHH jira for this issue that doesn't require a
specific implementation technique, so we can track this issue.

Scott

[1] http://lists.jboss.org/pipermail/hibernate-dev/2016-February/014219.html

>
> The inability to change javassist versions could be a major pain for
> Hibernate later on, as it may mean that older hibernate versions fail to
> work with newer JDK's if changes are made to the class file format.
>
> Stuart
>
> On Fri, 12 Feb 2016 at 03:03 Scott Marlow <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     As previously discussed, Hibernate applications need access to the
>     Javassist runtime classes (see example [1] enhanced application entity
>     if you didn't know this :).  A proposal was discussed on the
>     hibernate-dev mailing list that I think is the best short term solution.
>        I wanted to raise this issue here also, as I would like to later
>     create a pull request to bring in a new Hibernate ORM that includes this
>     change.  So, getting early feedback before we create JIRAs for the work,
>     is important.
>
>     The proposal is to private package (or shade), the Javassist classes, so
>     that Hibernate ORM has its own copy of the Javassist classes.  On
>     WildFly, we still would include Javassist for the other components that
>     use it and for Hibernate applications that have "build-time enhanced
>     entity classes" by an earlier Hibernate release.
>
>     One downside of this change is that Hibernate applications cannot easily
>     switch to a different version of the Javassist classes.
>
>     Another downside is that applications that depend on an older Hibernate
>     ORM version that includes "build-time enhanced entity classes", will
>     need to be cracked open, to add dependencies on the Javassist module
>     (since we will stop automatically adding Javassist to JPA application
>     deployments).
>
>     The advantage of this change, is that Hibernate applications can include
>     their own version of Javassist.
>
>     This will also have an impact on Hibernate build-time enhancing of
>     entity classes (e.g. enhanced bytecode will no longer depend on the
>     public Javassist classes).
>
>     Scott
>
>     [1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
>     _______________________________________________
>     wildfly-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Stuart Douglas


On Fri, 12 Feb 2016 at 07:41 Scott Marlow <[hidden email]> 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.

If they do go down the shade route will this shaded hibernate+javassist be a different artifact (i.e. will they still publish a non javassist version of hibernate)?

Stuart
 

If we use a private packaged copy of the jars, in order to use a
different version of Javassist, users would have to get a new version of
Hibernate that is built with that different Javassist (as you point out).

I would like to create a HHH jira for this issue that doesn't require a
specific implementation technique, so we can track this issue.

Scott

[1] http://lists.jboss.org/pipermail/hibernate-dev/2016-February/014219.html

>
> The inability to change javassist versions could be a major pain for
> Hibernate later on, as it may mean that older hibernate versions fail to
> work with newer JDK's if changes are made to the class file format.
>
> Stuart
>
> On Fri, 12 Feb 2016 at 03:03 Scott Marlow <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     As previously discussed, Hibernate applications need access to the
>     Javassist runtime classes (see example [1] enhanced application entity
>     if you didn't know this :).  A proposal was discussed on the
>     hibernate-dev mailing list that I think is the best short term solution.
>        I wanted to raise this issue here also, as I would like to later
>     create a pull request to bring in a new Hibernate ORM that includes this
>     change.  So, getting early feedback before we create JIRAs for the work,
>     is important.
>
>     The proposal is to private package (or shade), the Javassist classes, so
>     that Hibernate ORM has its own copy of the Javassist classes.  On
>     WildFly, we still would include Javassist for the other components that
>     use it and for Hibernate applications that have "build-time enhanced
>     entity classes" by an earlier Hibernate release.
>
>     One downside of this change is that Hibernate applications cannot easily
>     switch to a different version of the Javassist classes.
>
>     Another downside is that applications that depend on an older Hibernate
>     ORM version that includes "build-time enhanced entity classes", will
>     need to be cracked open, to add dependencies on the Javassist module
>     (since we will stop automatically adding Javassist to JPA application
>     deployments).
>
>     The advantage of this change, is that Hibernate applications can include
>     their own version of Javassist.
>
>     This will also have an impact on Hibernate build-time enhancing of
>     entity classes (e.g. enhanced bytecode will no longer depend on the
>     public Javassist classes).
>
>     Scott
>
>     [1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
>     _______________________________________________
>     wildfly-dev mailing list
>     [hidden email] <mailto:[hidden email]>
>     https://lists.jboss.org/mailman/listinfo/wildfly-dev
>

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow


On 02/11/2016 05:40 PM, Stuart Douglas wrote:

>
>
> On Fri, 12 Feb 2016 at 07:41 Scott Marlow <[hidden email]
> <mailto:[hidden email]>> 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.  :)

>
> If they do go down the shade route will this shaded hibernate+javassist
> be a different artifact (i.e. will they still publish a non javassist
> version of hibernate)?

Good question, I'm not really sure.

>
> Stuart
>
>
>     If we use a private packaged copy of the jars, in order to use a
>     different version of Javassist, users would have to get a new version of
>     Hibernate that is built with that different Javassist (as you point
>     out).
>
>     I would like to create a HHH jira for this issue that doesn't require a
>     specific implementation technique, so we can track this issue.
>
>     Scott
>
>     [1]
>     http://lists.jboss.org/pipermail/hibernate-dev/2016-February/014219.html
>
>      >
>      > The inability to change javassist versions could be a major pain for
>      > Hibernate later on, as it may mean that older hibernate versions
>     fail to
>      > work with newer JDK's if changes are made to the class file format.
>      >
>      > Stuart
>      >
>      > On Fri, 12 Feb 2016 at 03:03 Scott Marlow <[hidden email]
>     <mailto:[hidden email]>
>      > <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>      >
>      >     As previously discussed, Hibernate applications need access
>     to the
>      >     Javassist runtime classes (see example [1] enhanced
>     application entity
>      >     if you didn't know this :).  A proposal was discussed on the
>      >     hibernate-dev mailing list that I think is the best short
>     term solution.
>      >        I wanted to raise this issue here also, as I would like to
>     later
>      >     create a pull request to bring in a new Hibernate ORM that
>     includes this
>      >     change.  So, getting early feedback before we create JIRAs
>     for the work,
>      >     is important.
>      >
>      >     The proposal is to private package (or shade), the Javassist
>     classes, so
>      >     that Hibernate ORM has its own copy of the Javassist classes.  On
>      >     WildFly, we still would include Javassist for the other
>     components that
>      >     use it and for Hibernate applications that have "build-time
>     enhanced
>      >     entity classes" by an earlier Hibernate release.
>      >
>      >     One downside of this change is that Hibernate applications
>     cannot easily
>      >     switch to a different version of the Javassist classes.
>      >
>      >     Another downside is that applications that depend on an older
>     Hibernate
>      >     ORM version that includes "build-time enhanced entity
>     classes", will
>      >     need to be cracked open, to add dependencies on the Javassist
>     module
>      >     (since we will stop automatically adding Javassist to JPA
>     application
>      >     deployments).
>      >
>      >     The advantage of this change, is that Hibernate applications
>     can include
>      >     their own version of Javassist.
>      >
>      >     This will also have an impact on Hibernate build-time
>     enhancing of
>      >     entity classes (e.g. enhanced bytecode will no longer depend
>     on the
>      >     public Javassist classes).
>      >
>      >     Scott
>      >
>      >     [1] https://gist.github.com/scottmarlow/dc7ebfea654984f84e2e
>      >     _______________________________________________
>      >     wildfly-dev mailing list
>      > [hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email]
>     <mailto:[hidden email]>>
>      > https://lists.jboss.org/mailman/listinfo/wildfly-dev
>      >
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
>>
>>      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
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Steve Ebersole
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 <[hidden email]> 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
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Stuart Douglas
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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>> 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
>         [hidden email] <mailto:[hidden email]>
>
>         https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Sanne Grinovero
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 <[hidden email]> 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 <[hidden email]
>> <mailto:[hidden email]>> 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 <[hidden email]
>>     <mailto:[hidden email]>> 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
>>         [hidden email] <mailto:[hidden email]>
>>
>>         https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Gunnar Morling
My branch at https://github.com/gunnarmorling/hibernate-orm/tree/HHH-10536
is exactly doing this (using ByteBuddy).

Generated proxy types invoke java.lang.reflect.InvocationHandler, i.e.
no dependency to any library. Of course the same could be done either
using ASM directly or ripping ProxyFactory out of Javassist and adapt
it to do the same.

--Gunnar


2016-02-18 12:11 GMT+01:00 Sanne Grinovero <[hidden email]>:

> 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 <[hidden email]> 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 <[hidden email]
>>> <mailto:[hidden email]>> 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 <[hidden email]
>>>     <mailto:[hidden email]>> 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
>>>         [hidden email] <mailto:[hidden email]>
>>>
>>>         https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Carlo de Wolf
In reply to this post by Sanne Grinovero
Looks to me like Hibernate is exposing bad proxies to the user.

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.

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 <[hidden email]> 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 <[hidden email]
>>> <mailto:[hidden email]>> 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 <[hidden email]
>>>      <mailto:[hidden email]>> 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
>>>          [hidden email] <mailto:[hidden email]>
>>>
>>>          https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Steve Ebersole
Guys chill about the shading :)  That was only ever mentioned as a preference over the original suggestion to copy large number of classes from Javassist into Hibernate source.  I simply said that if we are copying these classes from Javassist that shading would be a better option for achieving that.  Neither of those are really good options, but forced to choose between the 2 I'd chose shading.

I think we all agree that (if possible) Hibernate not using using Javassist at all beyond actually generating the proxy and/or enhancing would be the best option.  So if this is possible lets go that route.

On Thu, Feb 18, 2016 at 12:17 PM Carlo de Wolf <[hidden email]> wrote:
Looks to me like Hibernate is exposing bad proxies to the user.

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.

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 <[hidden email]> 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 <[hidden email]
>>> <mailto:[hidden email]>> 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 <[hidden email]
>>>      <mailto:[hidden email]>> 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
>>>          [hidden email] <mailto:[hidden email]>
>>>
>>>          https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Steve Ebersole
In reply to this post by Gunnar Morling
This is definitely an interesting option.  I actually think the same could be achieved with Javassist too however.  That said, Javassist definitely has drawbacks.  I am not familiar with ByteBuddy.


On Thu, Feb 18, 2016 at 5:35 AM Gunnar Morling <[hidden email]> wrote:
My branch at https://github.com/gunnarmorling/hibernate-orm/tree/HHH-10536
is exactly doing this (using ByteBuddy).

Generated proxy types invoke java.lang.reflect.InvocationHandler, i.e.
no dependency to any library. Of course the same could be done either
using ASM directly or ripping ProxyFactory out of Javassist and adapt
it to do the same.

--Gunnar


2016-02-18 12:11 GMT+01:00 Sanne Grinovero <[hidden email]>:
> 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 <[hidden email]> 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 <[hidden email]
>>> <mailto:[hidden email]>> 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 <[hidden email]
>>>     <mailto:[hidden email]>> 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
>>>         [hidden email] <mailto:[hidden email]>
>>>
>>>         https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: private packaging Javassist jar in Hibernate ORM, so applications can have their own Javassist jar...

Scott Marlow
In reply to this post by Sanne Grinovero

On 02/18/2016 06:11 AM, 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 don't really like discussing the same issue on multiple lists but I
thought it made sense to ask here for further input (especially since I
would like to create a PR for the updated ORM that solves this problem
and having some buy in now that the PR will get merged, is helpful). :)

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

 From a timing point of view, it seems to me that it will likely take a
while before a future release of Hibernate ORM addresses this.  If I am
wrong about that, great.  But, I think that leaves the following options
for WildFly 10.1.0 and the proposed pr [1]:

A. Remove the private API label from the WildFly javassist static module
so that Hibernate native applications can depend on Javassist without
fear that their application may fail to deploy in the future because of
the Javassist dependency.

B.  Stick with the current [1] hack of exporting the Javassist
dependency to applications that have a dependency on the Hibernate ORM
module.

Scott

[1] https://github.com/wildfly/wildfly/pull/8474 which has a sad "fix
me" label.
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
12