Since [proxy] is in proper, Romain probably doesn't (yet) have access to commit this himself.
Matt On Wed, Jul 31, 2013 at 2:31 PM, James Carman <ja...@carmanconsulting.com>wrote: > Any chance you could hook that into our existing test suite? We have > a base class for all ProxyFactory tests. Also, could you apply your > fix to the 2.0 branch? We're not upgrading proxy-1.x to Java 6. > That's happening in the 2.x release. > > On Wed, Jul 31, 2013 at 3:07 PM, Romain Manni-Bucau > <rmannibu...@gmail.com> wrote: > > Hi > > > > here is the asm4-shaded impl: > https://gist.github.com/rmannibucau/6125125 > > > > *Romain Manni-Bucau* > > *Twitter: @rmannibucau <https://twitter.com/rmannibucau>* > > *Blog: **http://rmannibucau.wordpress.com/*< > http://rmannibucau.wordpress.com/> > > *LinkedIn: **http://fr.linkedin.com/in/rmannibucau* > > *Github: https://github.com/rmannibucau* > > > > > > > > 2013/7/29 Romain Manni-Bucau <rmannibu...@gmail.com> > > > >> hmm not sure i follow, here we don't shade asm (it is already done) and > if > >> all libs shade it we will have at least 5 shade of the same version in > >> tomee for instance (same comment on the app side) so that's not a > solution > >> for each lib. [proxy] is small enough to not shade IMO. That said if > your > >> relocation trick works it would be enough to copy classes with > relocation > >> in 3-4 places. > >> > >> *Romain Manni-Bucau* > >> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>* > >> *Blog: **http://rmannibucau.wordpress.com/*< > http://rmannibucau.wordpress.com/> > >> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau* > >> *Github: https://github.com/rmannibucau* > >> > >> > >> > >> 2013/7/29 Matt Benson <gudnabr...@gmail.com> > >> > >>> Rather than duplicating code I thought we could code to asm4's released > >>> jars, and provide the basic proxy-asm artifact. Then shade asm4 and > >>> provide proxy-asm-shaded. Then optionally, we could create another > shaded > >>> jar that relocates to the same destination as xbean-shaded-asm4 but > does > >>> not actually shade the classes. I think maven-shade-plugin would do > this > >>> by specifying relocations without the artifactSet, though I haven't > tried > >>> it. Then we support: > >>> > >>> * asm4 is on classpath > >>> * one well-known shading that the user may already have on the > classpath > >>> * dependencies shaded to a namespace specific to proxy-asm > >>> > >>> One of these options will work in every case. Even ASM's own FAQ > >>> recommends the equivalent of shading per consuming library[1]. > >>> > >>> Matt > >>> > >>> [1] http://asm.ow2.org/doc/faq.html#Q15 > >>> > >>> > >>> On Mon, Jul 29, 2013 at 9:59 AM, Romain Manni-Bucau < > >>> rmannibu...@gmail.com> wrote: > >>> > >>>> You have the clean proxy code here (just rework the method generation > >>>> which is a bit different): > >>>> > http://svn.apache.org/repos/asf/tomee/tomee/trunk/container/openejb-core/src/main/java/org/apache/openejb/util/proxy/LocalBeanProxyFactory.java > >>>> > >>>> the point is i already have cases where i want to use asm and asm > >>>> shaded, we can multiply the impl number too but it would duplicate > the code. > >>>> > >>>> About the perf a bench would say it, i didn't take time to do it. > >>>> > >>>> *Romain Manni-Bucau* > >>>> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>* > >>>> *Blog: **http://rmannibucau.wordpress.com/*< > http://rmannibucau.wordpress.com/> > >>>> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau* > >>>> *Github: https://github.com/rmannibucau* > >>>> > >>>> > >>>> > >>>> 2013/7/29 Matt Benson <gudnabr...@gmail.com> > >>>> > >>>>> > >>>>> On Sun, Jul 28, 2013 at 12:16 PM, Romain Manni-Bucau < > >>>>> rmannibu...@gmail.com> wrote: > >>>>> > >>>>>> answers inline > >>>>>> > >>>>>> *Romain Manni-Bucau* > >>>>>> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>* > >>>>>> *Blog: **http://rmannibucau.wordpress.com/*< > http://rmannibucau.wordpress.com/> > >>>>>> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau* > >>>>>> *Github: https://github.com/rmannibucau* > >>>>>> > >>>>>> > >>>>>> > >>>>>> 2013/7/28 Matt Benson <gudnabr...@gmail.com> > >>>>>> > >>>>>>> Interesting patch. I have some questions and comments: > >>>>>>> > >>>>>>> - You'd additionally need to make sure the impl class is non-final, > >>>>>>> no? > >>>>>>> > >>>>>> > >>>>>> hmm, good question i didn't check but with asm we can subclass final > >>>>>> classes, hehe > >>>>>> > >>>>> > >>>>> > >>>>> We can? How devious... well, then strike my question. > >>>>> > >>>>> > >>>>>> > >>>>>> > >>>>>>> - note to others that asm4-shaded is used because asm didn't change > >>>>>>> packages from v3. Good to see this in use; I hadn't kept track > after > >>>>>>> submitting that patch. ;-) > >>>>>>> > >>>>>> > >>>>>> i used asm4 since that's the more up to date and it supports java 7 > >>>>>> very well. The shade was used since provided in tomee and owb but > real asm > >>>>>> should be fine (see next point) > >>>>>> > >>>>>> > >>>>>>> - Would you explain the purpose of the AsmFacade class? Much of the > >>>>>>> "nuts and bolts" work of the patch seems quite different from what > I > >>>>>>> perceive as "typical asm client code." > >>>>>>> > >>>>>> > >>>>>> i first wrote it with asm imports but a common issue is: do i use > asm? > >>>>>> spring-asm-shade? xbean-asm-shade? so AsmFacade is an utility class > to > >>>>>> allow to use whatever impl is here (almost). > >>>>>> > >>>>>> > >>>>> > >>>>> While I find this to be interesting and quite clever, I feel like > it's > >>>>> maybe too much. For one point, have you tried searching the web for > >>>>> meaningful examples of ASM code? It's not that easy IMO. I think > it'd be > >>>>> nicer for our ASM code to exemplify "normal" ASM as much as > possible. I'd > >>>>> say it'd be enough to write the basic impl against stock asm4. If we > >>>>> wanted we could then provide one artifact that shades asm4, and > another > >>>>> that rewrites the compiled classes to depend on xbean-shaded-asm4, > and > >>>>> surely that would be enough for users to get by with. Then our code > would > >>>>> be more intelligible as well as useful from the perspective of > helping > >>>>> other devs learn from good examples. > >>>>> > >>>>> Back to the subject of cglib, do you expect this implementation > should > >>>>>>> significantly outperform it for any reason ( if so, which? ), or > is the > >>>>>>> main motivation that cglib is almost dead as you say? > >>>>>>> > >>>>>> > >>>>>> since cglib is dead we need something else and i expect the impl to > be > >>>>>> faster than javassist. Another nice side effect is no dep in a > container > >>>>>> providing asm. > >>>>>> > >>>>>> > >>>>> > >>>>> I am taking this as still saying, yes, the ASM proxy implementation > >>>>> might not be any faster than cglib. ;) Which is fine. > >>>>> > >>>>> Thanks! > >>>>> > >>>>> Matt > >>>>> > >>>>> > >>>>>> Thanks and regards, > >>>>>>> Matt > >>>>>>> On Jul 28, 2013 10:58 AM, "Romain Manni-Bucau" < > rmannibu...@gmail.com> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Hi > >>>>>>>> > >>>>>>>> here is a patch implementing proxying using ASM: > >>>>>>>> https://gist.github.com/rmannibucau/6099063 > >>>>>>>> > >>>>>>>> having the handlers used by default in ProxyFactory protected > would > >>>>>>>> avoid to copy them in ASMProxyFactory. > >>>>>>>> > >>>>>>>> *Romain Manni-Bucau* > >>>>>>>> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>* > >>>>>>>> *Blog: **http://rmannibucau.wordpress.com/*< > http://rmannibucau.wordpress.com/> > >>>>>>>> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau* > >>>>>>>> *Github: https://github.com/rmannibucau* > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> 2013/7/28 Romain Manni-Bucau <rmannibu...@gmail.com> > >>>>>>>> > >>>>>>>>> Cglib is "almost" dead if i'm right, javassist is alive but not > >>>>>>>>> that stable and owb is faster ATM and at least would bring an > Apache impl > >>>>>>>>> adapted to [proxy]. > >>>>>>>>> > >>>>>>>>> Note: the fact to be able to reuse InvocationHandler and not a > new > >>>>>>>>> API is great too > >>>>>>>>> Le 27 juil. 2013 20:13, "Matt Benson" <gudnabr...@gmail.com> a > >>>>>>>>> écrit : > >>>>>>>>> > >>>>>>>>> AFAIK Mark Struberg's work on the OWB proxies could be > instructive, > >>>>>>>>>> and > >>>>>>>>>> since I've just spent several weeks in ASM hell I might just be > a > >>>>>>>>>> bit of > >>>>>>>>>> use there myself. The only thing is, isn't cglib built on ASM as > >>>>>>>>>> well? The > >>>>>>>>>> dynamic nature of the various proxy helpers means that we > probably > >>>>>>>>>> couldn't > >>>>>>>>>> really improve on cglib, i.e. only if we could test invocation > >>>>>>>>>> matching up > >>>>>>>>>> front while creating the proxy class would we be faster. > >>>>>>>>>> > >>>>>>>>>> Matt > >>>>>>>>>> On Jul 27, 2013 12:22 PM, "Romain Manni-Bucau" < > >>>>>>>>>> rmannibu...@gmail.com> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> > Hehe, we benched in owb but lets wait the porting ;) > >>>>>>>>>> > Le 27 juil. 2013 16:49, "James Carman" < > >>>>>>>>>> ja...@carmanconsulting.com> a > >>>>>>>>>> > écrit : > >>>>>>>>>> > > >>>>>>>>>> > > On Sat, Jul 27, 2013 at 10:34 AM, Romain Manni-Bucau > >>>>>>>>>> > > <rmannibu...@gmail.com> wrote: > >>>>>>>>>> > > > Once ill have done the monitoring stuff ill try to work on > >>>>>>>>>> it. > >>>>>>>>>> > > > >>>>>>>>>> > > What would be really cool is to have a "smackdown" once we > get > >>>>>>>>>> ASM > >>>>>>>>>> > > into the mix to see which one performs the best and exactly > >>>>>>>>>> how fast > >>>>>>>>>> > > they are compared to one another. > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > --------------------------------------------------------------------- > >>>>>>>>>> > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >>>>>>>>>> > > For additional commands, e-mail: > dev-h...@commons.apache.org > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>>> > >>>> > >>> > >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >