Hi, Have you noticed a perf difference in your application with and without the patch? I am wondering if modern VMs have catched up with what Javassist does. On 26 mai 2010, at 18:29, Кирилл Кленский wrote: > 1. I have noticed that > org.hibernate.bytecode.javassist.BulkAccessorFactory.findAccessors(...) is > searching for accessor methods in the optimized entity class only. This > means that the methods from the superclasses are not visible during > BulkAccessor creation unless overridden by child classes. By enhancing the > algorithm to search down the inheritance tree we could avoid creation of > redundant methods which increase the code verbosity a lot. In our case > almost all the entities are inherited from the base classes having the > common entity properties defined, so the reflection optimization does not > work for any of them until we override the inherited methods in all the > child classes. The implementation is trivial, but I have got a ready > prototype if anybody is interested.

I ran this same exact comparison before and I seem to recall much different results. Unfortunately I no longer have that code. This was part of http://opensource.atlassian.com/projects/hibernate/browse/HHH-227 Can you make sure you "prime" or "warm up" the jvm before you start taking measurements? On Thu, 2010-05-27 at 15:39 +0300, Кирилл Кленский wrote: > Hi, > > My measurements have indicated that there is a performance gain. I have > measured the time spent in setPropertyValues and getPropertyValues. > The optimized version was 4 times faster in these methods giving an > estimated application performance increase of about 3%. > Optimizing getPropertyValue and setPropertyValue could give 1.5% more > according to our rough calculations. > > Kirill > > 26 мая 2010 г. 23:53 пользователь Emmanuel Bernard > <emmanuel(a)hibernate.xn--org&gt;-8fga3f8au1a3a: > >> Hi, >> Have you noticed a perf difference in your application with and without the >> patch? >> I am wondering if modern VMs have catched up with what Javassist does. >> >> On 26 mai 2010, at 18:29, Кирилл Кленский wrote: >> >>> 1. I have noticed that >>> org.hibernate.bytecode.javassist.BulkAccessorFactory.findAccessors(...) >> is >>> searching for accessor methods in the optimized entity class only. This >>> means that the methods from the superclasses are not visible during >>> BulkAccessor creation unless overridden by child classes. By enhancing >> the >>> algorithm to search down the inheritance tree we could avoid creation of >>> redundant methods which increase the code verbosity a lot. In our case >>> almost all the entities are inherited from the base classes having the >>> common entity properties defined, so the reflection optimization does not >>> work for any of them until we override the inherited methods in all the >>> child classes. The implementation is trivial, but I have got a ready >>> prototype if anybody is interested. >> > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev

Perhaps it short circuits those copies and other overheads if no security manager is defined (ala as in my IDE). That would explain how I can see minimal improvement while Kirill sees a 4x improvement. Still rather confirm these numbers are accurate. Kirill? On Thu, 2010-05-27 at 10:12 -0400, Bill Burke wrote: > Carlo deWolfe found that one of the big perf problems with Java > reflection is that it is constantly doing security checks with the > security manager and every get/find request makes a copy of the > method/field objects. He had a hack for this, but you'll have to > consult him on what it is. The JBoss Reflections project might have it. > > I think once this hack is intiated, it is an improvement over Javassist. > If you think about it, Java VM has to build up this information anyways... > > Steve Ebersole wrote: > > I ran this same exact comparison before and I seem to recall much > > different results. Unfortunately I no longer have that code. This was > > part of > > http://opensource.atlassian.com/projects/hibernate/browse/HHH-227 > > > > Can you make sure you "prime" or "warm up" the jvm before you start > > taking measurements? > > > > On Thu, 2010-05-27 at 15:39 +0300, Кирилл Кленский wrote: > >> Hi, > >> > >> My measurements have indicated that there is a performance gain. I have > >> measured the time spent in setPropertyValues and getPropertyValues. > >> The optimized version was 4 times faster in these methods giving an > >> estimated application performance increase of about 3%. > >> Optimizing getPropertyValue and setPropertyValue could give 1.5% more > >> according to our rough calculations. > >> > >> Kirill > >> > >> 26 мая 2010 г. 23:53 пользователь Emmanuel Bernard > >> <emmanuel(a)hibernate.xn--org&gt;-8fga3f8au1a3a: > >> > >>> Hi, > >>> Have you noticed a perf difference in your application with and without the > >>> patch? > >>> I am wondering if modern VMs have catched up with what Javassist does. > >>> > >>> On 26 mai 2010, at 18:29, Кирилл Кленский wrote: > >>> > >>>> 1. I have noticed that > >>>> org.hibernate.bytecode.javassist.BulkAccessorFactory.findAccessors(...) > >>> is > >>>> searching for accessor methods in the optimized entity class only. This > >>>> means that the methods from the superclasses are not visible during > >>>> BulkAccessor creation unless overridden by child classes. By enhancing > >>> the > >>>> algorithm to search down the inheritance tree we could avoid creation of > >>>> redundant methods which increase the code verbosity a lot. In our case > >>>> almost all the entities are inherited from the base classes having the > >>>> common entity properties defined, so the reflection optimization does not > >>>> work for any of them until we override the inherited methods in all the > >>>> child classes. The implementation is trivial, but I have got a ready > >>>> prototype if anybody is interested. > >>> > >> _______________________________________________ > >> hibernate-dev mailing list > >> hibernate-dev(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > > > -- Steve Ebersole <steve(a)hibernate.org&gt; http://hibernate.org

Perhaps. We do in fact utilize setAccessible in the Hibernate code and so when I did performance testing I did that as well as it most closely matched our actual usage. Most likely it applies the checks "up front" when setAccessible is called. I'll take a look at the test and the numbers when i have a chance (probably not till next week), but it is difficult to mimic all the variables that come into play in different server environments (using server versus client is only a small part). On Wed, 2010-06-02 at 13:24 +0300, Кирилл Кленский wrote: > Hi, > > > I have done some tests that are more pure in that they do not involve > the application logic. > > > Java version: jdk1.6.0_19 32-bit > Results include both client and server mode tests. Numbers represent > 100 000 000 invocations time in milliseconds. > > > The result format is the following: > server_mode_test_time(server_mode_warmup_time) - > client_mode_test_time(client_mode_warmup_time) > > > ========================== > Method access with direct call > 24(4) - 69(70) > > > Method access with reflection > 815(3893) - 36 969(39162) > > > Method access with reflection and setAccessible(true) > 339(640) - 1297(1356) > > > Method access with generated bytecode > 3(2) - 646(666) > > > Field access with reflection > 40333(40016) - 40333(58800) > > > Field access with reflection and setAccessible(true) > 729(786) - 729(7696) > ========================== > > > I have the following thoughts after all: > > > 1. Generated bytecode is comparable to direct access. Strange enough > that it was even faster than direct access in server mode. Tests show > that JVM seems to optimize reflection calls (warmup helps) but the > difference between reflection and direct calls is huge. > 2. Setting Method.setAccessible() and Field.setAccessible() to true > helps to avoid security checks and increases the performance. May be > this is the trick Bill is talking about? > This potentially could be a relatively easy way to increase the > performance especially if JVM is running in client mode (method > invocation security checks take approx 10 times more time in client > mode compared to server). In server mode the increase is relatively > small for methods (several times) and huge for fields (approx 50 > times). > > > I have attached the test code if somebody wants to compare results. > Javassist 3.8.0.GA was used to generate bytecode and is required to > build/run the test. > > > Regards, > Kirill Klenski > > > 2010/5/27 Steve Ebersole <steve(a)hibernate.org&gt; > Perhaps it short circuits those copies and other overheads if > no > security manager is defined (ala as in my IDE). That would > explain how > I can see minimal improvement while Kirill sees a 4x > improvement. > > Still rather confirm these numbers are accurate. Kirill? > > > On Thu, 2010-05-27 at 10:12 -0400, Bill Burke wrote: > > Carlo deWolfe found that one of the big perf problems with > Java > > reflection is that it is constantly doing security checks > with the > > security manager and every get/find request makes a copy of > the > > method/field objects. He had a hack for this, but you'll > have to > > consult him on what it is. The JBoss Reflections project > might have it. > > > > I think once this hack is intiated, it is an improvement > over Javassist. > > If you think about it, Java VM has to build up this > information anyways... > > > > Steve Ebersole wrote: > > > I ran this same exact comparison before and I seem to > recall much > > > different results. Unfortunately I no longer have that > code. This was > > > part of > > > > http://opensource.atlassian.com/projects/hibernate/browse/HHH-227 > > > > > > Can you make sure you "prime" or "warm up" the jvm before > you start > > > taking measurements? > > > > > > On Thu, 2010-05-27 at 15:39 +0300, Кирилл Кленский wrote: > > >> Hi, > > >> > > >> My measurements have indicated that there is a > performance gain. I have > > >> measured the time spent in setPropertyValues and > getPropertyValues. > > >> The optimized version was 4 times faster in these methods > giving an > > >> estimated application performance increase of about 3%. > > >> Optimizing getPropertyValue and setPropertyValue could > give 1.5% more > > >> according to our rough calculations. > > >> > > >> Kirill > > >> > > >> 26 мая 2010 г. 23:53 пользователь Emmanuel Bernard > > >> <emmanuel(a)hibernate.xn--org&gt;-8fga3f8au1a3a: > > >> > > >>> Hi, > > >>> Have you noticed a perf difference in your application > with and without the > > >>> patch? > > >>> I am wondering if modern VMs have catched up with what > Javassist does. > > >>> > > >>> On 26 mai 2010, at 18:29, Кирилл Кленский wrote: > > >>> > > >>>> 1. I have noticed that > > >>>> > org.hibernate.bytecode.javassist.BulkAccessorFactory.findAccessors(...) > > >>> is > > >>>> searching for accessor methods in the optimized entity > class only. This > > >>>> means that the methods from the superclasses are not > visible during > > >>>> BulkAccessor creation unless overridden by child > classes. By enhancing > > >>> the > > >>>> algorithm to search down the inheritance tree we could > avoid creation of > > >>>> redundant methods which increase the code verbosity a > lot. In our case > > >>>> almost all the entities are inherited from the base > classes having the > > >>>> common entity properties defined, so the reflection > optimization does not > > >>>> work for any of them until we override the inherited > methods in all the > > >>>> child classes. The implementation is trivial, but I > have got a ready > > >>>> prototype if anybody is interested. > > >>> > > >> _______________________________________________ > > >> hibernate-dev mailing list > > >> hibernate-dev(a)lists.jboss.org > > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > > > > > > > > > -- > > > Steve Ebersole <steve(a)hibernate.org&gt; > http://hibernate.org > > > > -- Steve Ebersole <steve(a)hibernate.org&gt; http://hibernate.org

Ok, now that is reasonable :) And looking through the source (for Field at least) is does circumvent a lot of checks if accessible is set. So I can move the '!ReflectHelper.isPublic' check into the catch blocks to continue to error in cases where we *need* to call setAccessible; for other cases I'd just warn: if ( !ReflectHelper.isPublic(theClass, method) ) { method.setAccessible(true); } becomes: if ( !method.isAccessible() ) { try { method.setAccessible( true ); } catch ( SecurityException se ) { if ( !ReflectHelper.isPublic( theClass, method ) ) { throw se; } else { log.warn( ... ); } } } Could you please create a JIRA? On Wed, 2010-06-02 at 17:44 +0300, Кирилл Кленский wrote: > "We do in fact utilize setAccessible in the Hibernate code" > > > Yes, but if I am not mistaken only if the method/field is not public. > From code: !ReflectHelper.isPublic(theClass, method) ) > method.setAccessible(true); > We could increase performance easily if we call setAccessible even for > public methods - we get rid of costly security checks. > > > Kirill > > > 2 июня 2010 г. 16:57 пользователь Steve Ebersole <steve(a)hibernate.org&gt; > написал: > Perhaps. We do in fact utilize setAccessible in the Hibernate > code and > so when I did performance testing I did that as well as it > most closely > matched our actual usage. Most likely it applies the checks > "up front" > when setAccessible is called. > > I'll take a look at the test and the numbers when i have a > chance > (probably not till next week), but it is difficult to mimic > all the > variables that come into play in different server environments > (using > server versus client is only a small part). > > > On Wed, 2010-06-02 at 13:24 +0300, Кирилл Кленский wrote: > > Hi, > > > > > > I have done some tests that are more pure in that they do > not involve > > the application logic. > > > > > > Java version: jdk1.6.0_19 32-bit > > Results include both client and server mode tests. Numbers > represent > > 100 000 000 invocations time in milliseconds. > > > > > > The result format is the following: > > server_mode_test_time(server_mode_warmup_time) - > > client_mode_test_time(client_mode_warmup_time) > > > > > > ========================== > > Method access with direct call > > 24(4) - 69(70) > > > > > > Method access with reflection > > 815(3893) - 36 969(39162) > > > > > > Method access with reflection and setAccessible(true) > > 339(640) - 1297(1356) > > > > > > Method access with generated bytecode > > 3(2) - 646(666) > > > > > > Field access with reflection > > 40333(40016) - 40333(58800) > > > > > > Field access with reflection and setAccessible(true) > > 729(786) - 729(7696) > > ========================== > > > > > > I have the following thoughts after all: > > > > > > 1. Generated bytecode is comparable to direct access. > Strange enough > > that it was even faster than direct access in server mode. > Tests show > > that JVM seems to optimize reflection calls (warmup helps) > but the > > difference between reflection and direct calls is huge. > > 2. Setting Method.setAccessible() and Field.setAccessible() > to true > > helps to avoid security checks and increases the > performance. May be > > this is the trick Bill is talking about? > > This potentially could be a relatively easy way to increase > the > > performance especially if JVM is running in client mode > (method > > invocation security checks take approx 10 times more time in > client > > mode compared to server). In server mode the increase is > relatively > > small for methods (several times) and huge for fields > (approx 50 > > times). > > > > > > I have attached the test code if somebody wants to compare > results. > > Javassist 3.8.0.GA was used to generate bytecode and is > required to > > build/run the test. > > > > > > Regards, > > Kirill Klenski > > > > > > 2010/5/27 Steve Ebersole <steve(a)hibernate.org&gt; > > Perhaps it short circuits those copies and other > overheads if > > no > > security manager is defined (ala as in my IDE). > That would > > explain how > > I can see minimal improvement while Kirill sees a 4x > > improvement. > > > > Still rather confirm these numbers are accurate. > Kirill? > > > > > > On Thu, 2010-05-27 at 10:12 -0400, Bill Burke wrote: > > > Carlo deWolfe found that one of the big perf > problems with > > Java > > > reflection is that it is constantly doing security > checks > > with the > > > security manager and every get/find request makes > a copy of > > the > > > method/field objects. He had a hack for this, but > you'll > > have to > > > consult him on what it is. The JBoss Reflections > project > > might have it. > > > > > > I think once this hack is intiated, it is an > improvement > > over Javassist. > > > If you think about it, Java VM has to build up > this > > information anyways... > > > > > > Steve Ebersole wrote: > > > > I ran this same exact comparison before and I > seem to > > recall much > > > > different results. Unfortunately I no longer > have that > > code. This was > > > > part of > > > > > > > http://opensource.atlassian.com/projects/hibernate/browse/HHH-227 > > > > > > > > Can you make sure you "prime" or "warm up" the > jvm before > > you start > > > > taking measurements? > > > > > > > > On Thu, 2010-05-27 at 15:39 +0300, Кирилл > Кленский wrote: > > > >> Hi, > > > >> > > > >> My measurements have indicated that there is a > > performance gain. I have > > > >> measured the time spent in setPropertyValues > and > > getPropertyValues. > > > >> The optimized version was 4 times faster in > these methods > > giving an > > > >> estimated application performance increase of > about 3%. > > > >> Optimizing getPropertyValue and > setPropertyValue could > > give 1.5% more > > > >> according to our rough calculations. > > > >> > > > >> Kirill > > > >> > > > >> 26 мая 2010 г. 23:53 пользователь Emmanuel > Bernard > > > >> <emmanuel(a)hibernate.xn--org&gt;-8fga3f8au1a3a: > > > >> > > > >>> Hi, > > > >>> Have you noticed a perf difference in your > application > > with and without the > > > >>> patch? > > > >>> I am wondering if modern VMs have catched up > with what > > Javassist does. > > > >>> > > > >>> On 26 mai 2010, at 18:29, Кирилл Кленский > wrote: > > > >>> > > > >>>> 1. I have noticed that > > > >>>> > > > org.hibernate.bytecode.javassist.BulkAccessorFactory.findAccessors(...) > > > >>> is > > > >>>> searching for accessor methods in the > optimized entity > > class only. This > > > >>>> means that the methods from the superclasses > are not > > visible during > > > >>>> BulkAccessor creation unless overridden by > child > > classes. By enhancing > > > >>> the > > > >>>> algorithm to search down the inheritance tree > we could > > avoid creation of > > > >>>> redundant methods which increase the code > verbosity a > > lot. In our case > > > >>>> almost all the entities are inherited from > the base > > classes having the > > > >>>> common entity properties defined, so the > reflection > > optimization does not > > > >>>> work for any of them until we override the > inherited > > methods in all the > > > >>>> child classes. The implementation is trivial, > but I > > have got a ready > > > >>>> prototype if anybody is interested. > > > >>> > > > >> _______________________________________________ > > > >> hibernate-dev mailing list > > > >> hibernate-dev(a)lists.jboss.org > > > >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > > > > > > > > > > > > > > > -- > > > > > > Steve Ebersole <steve(a)hibernate.org&gt; > > http://hibernate.org > > > > > > > > > > > > -- > > Steve Ebersole <steve(a)hibernate.org&gt; > http://hibernate.org > > > -- Steve Ebersole <steve(a)hibernate.org&gt; http://hibernate.org