Ok... Well, I'm not sure I'm ready to go there just yet... I did want to send an update to whom it may concern...
I worked with our load test environment today, and was able to reproduce my problem fairly easily. We ran a set of about 20 virtual users in a script that would have all 20 request a page at the same time, wait for all 20 to get responses, then send all 20 at another page. All, on a freshly started server, with no page objects pooled yet. The repetitive method errors started showing up almost immediately with the 3.0.4 code. So, next I overloaded the getter in the engine that returns the DefaultComponentClassEnhancer, and had it return my own ComponentClassEnhancer. My version works the way 3.0.3 did, with the double-checked locking code. (I know this code is not guaranteed to work, but because we hadn't seen this problem before, it seemed like it had been working for us). I ran the same test with the double-checked locking code. No problems. (FYI - I was testing against a dual Xeon (NetBurst, not Core) server and the JRockit VM on RedHat Linux). Ok, so, this "fixes" my problem, but I've been studying up on this double-checked locking thing, and get that it isn't really safe to assume it will work... So, I tried doing the only thing that most of the articles I read said you could do - synchronize the whole thing. So I re-implemented my custom ComponentClassEnhancer to syncrhonize the method in question... Of course, this also works... It also made my simple 20 user test take 2 minutes longer. :( My test of course was a worst case, hitting new pages with nothing pooled. Tomorrow I'm going to try both solutions with a more realistic load, and find out if the synchronization solution is really going to hurt us or not. On 11/13/06, Jesse Kuhnert <[EMAIL PROTECTED]> wrote:
Yep, a much more efficient means of doing it would be to use http://dcl.mathcs.emory.edu/util/backport-util-concurrent/ . It seems like this is crossing over on the annotation exceptions being thrown when caching is disabled as well. (though maybe the solution for t4 won't be the same as t3 since t4 is based around hivemind. ) On 11/13/06, Nick Westgate <[EMAIL PROTECTED]> wrote: > > Hi Jeff. > > > hrm.. it almost seems that patch was intended to fix the very problem I > am > > seeing.. > > Indeed. > > > I'll look a little further, may be there is something I'm missing... > > The original code is definitely flawed. Unfortunately, it seems that > the new code: > synchronized(specification) > > is not sufficient to replace: > synchronized(this) > > which suggests synchronizing on "this" is necessary, or equivalently > removing the synchronization block and synchronizing the method. > > The only other way to address it would be at a higher level outside > this method, which would be nice to avoid the performance hit. > > Cheers, > Nick. > > > > On 11/12/06, andyhot <[EMAIL PROTECTED]> wrote: > >> > >> Jeff Poetker wrote: > >> > Ok, Looking at the status.xml file in that revision I see the > >> > following diff, are the changes in question related to "Double > checked > >> > locking bug prevents use of multi processor environments > >> (efficiently)."? > >> > >> > >> Yea, looks like it... > >> Here's the jira: > >> http://issues.apache.org/jira/browse/TAPESTRY-806 > >> It describes that exact change... > >> > >> Weird thing is that the issue is marked as 'affects version:3.0.5' and > >> 'fix version:3.0.5' > >> but from the date and this > >> > >> > http://svn.apache.org/viewvc/tapestry/tapestry4/tags/3.0.4/framework/src/org/apache/tapestry/enhance/DefaultComponentClassEnhancer.java?view=log > >> > >> and the code, it was indeed included in 3.0.4 > >> > >> > > >> > > >> > --- jakarta/tapestry/branches/branch-3-0/status.xml 2006/03/14 > >> > 13:43:09 385801 > >> > +++ jakarta/tapestry/branches/branch-3-0/status.xml 2006/03/14 > >> > 13:47:10 385802 > >> > @@ -11,6 +11,7 @@ > >> > <person name="Tsvetelin Saykov" id="TS"/> > >> > <person name="Neil Clayton" id="NC"/> > >> > <person name="Paul Ferraro" id="PF"/> > >> > + <person name="Jesse Kuhnert" id="JK" /> > >> > <!-- Retired --> > >> > <person name="Malcom Edgar" id="ME"/> > >> > <!-- Add more people here --> > >> > @@ -126,6 +127,21 @@ > >> > <changes> > >> > <release version="3.0.4" date="unreleased"> > >> > <action type="fix" dev="GL" fixes-bug="TAPESTRY-431"> Fixed > >> > TemplateParser throws an exception and stops parsing when duplicate > >> > attributes are found in a tag. </action> > >> > + <action type="fix" dev="JK" fixes-bug="TAPESTRY-877" > >> > due-to="Brian K. Wallace"> > >> > + Javassist url was incorrect. > >> > + </action> > >> > + <action type="remove" dev="JK" fixes-bug="TAPESTRY-878" > >> > due-to="Brian K. Wallace" > > >> > + Removed old tutorial example. > >> > + </action> > >> > + <action type="fix" dev="JK" fixes-bug="TAPESTRY-806" > >> > due-to="Nick Westgate" > > >> > + Double checked locking bug prevents use of multi processor > >> > environments (efficiently). > >> > + </action> > >> > + <action type="fix" dev="JK" fixes-bug="TAPESTRY-241" > >> > due-to="Kurtis Williams" > > >> > + binding for convertor needed to be inherited-binding > >> > + </action> > >> > + <action type="fix" dev="JK" fixes-bug="TAPESTRY-193" > >> > due-to="Brian K. Wallace" > > >> > + AssetService not resolving file prefixes correctly. > >> > + </action> > >> > </release> > >> > <release version="3.0.3" date="Mar 26 2005"> > >> > <action type="fix" dev="PF" fixes-bug="TAPESTRY-278"> Fixes > >> > security flaw in asset service. </action> > >> > > >> > > >> > On Nov 11, 2006, at 2:51 PM, andyhot wrote: > >> > > >> >> Here's the history of that file > >> >> > >> > http://svn.apache.org/viewvc/tapestry/tapestry4/branches/branch-3-0/framework/src/org/apache/tapestry/enhance/DefaultComponentClassEnhancer.java?view=log > >> > >> >> > >> >> and here's the diffs to the previous version > >> >> > >> > http://svn.apache.org/viewvc/tapestry/tapestry4/branches/branch-3-0/framework/src/org/apache/tapestry/enhance/DefaultComponentClassEnhancer.java?r1=243897&r2=385802 > >> > >> >> > >> >> > >> >> Jesse's change log states "Applied patches/fixed bugs" though i > >> >> couldn't find any related entry in JIRA during my brief research... > >> >> So, there seems to have been a reason for that change, perhaps Jesse > >> >> can shed more light. > >> >> > >> >> > >> >> > >> >> Jeff Poetker wrote: > >> >>> I work on a project that is using Tapestry 3. We're currently > >> >>> working on version 4 of this product, and in this release we have > >> >>> upgraded to version 3.0.4 of Tapestry. In every version of our > >> >>> product we have done some amount of load testing as part of our > >> >>> quality assurance process. > >> >>> > >> >>> In this release, we've started seeing this sort of exception a lot > >> >>> during load (and occasionally in our functional) testing. > >> >>> > >> >>> Tapestry exception > >> >>> java.lang.Throwable: Unable to define class > >> >>> org.apache.tapestry.link.PageLink$Enhance_32: > >> >>> org/apache/tapestry/link/PageLink$Enhance_32 (Repetitive method > >> >>> name/signature) > >> >>> > >> >>> at > >> >>> org.apache.tapestry.enhance.EnhancedClassLoader.defineClass( > >> EnhancedClassLoader.java:55) > >> >>> > >> >>> at > >> >>> > >> > org.apache.tapestry.enhance.javassist.EnhancedClass.createEnhancedSubclass > >> > >> (EnhancedClass.java:133) > >> >>> > >> >>> at > >> >>> > >> > org.apache.tapestry.enhance.ComponentClassFactory.createEnhancedSubclass ( > >> ComponentClassFactory.java:336) > >> >>> > >> >>> at > >> >>> > >> > org.apache.tapestry.enhance.DefaultComponentClassEnhancer.constructComponentClass > >> > >> (DefaultComponentClassEnhancer.java:139) > >> >>> > >> >>> > >> >>> at > >> >>> > >> > org.apache.tapestry.enhance.DefaultComponentClassEnhancer.getEnhancedClass > >> > >> (DefaultComponentClassEnhancer.java:94) > >> >>> > >> >>> > >> >>> at > >> >>> org.apache.tapestry.pageload.PageLoader.instantiateComponent( > >> PageLoader.java:603) > >> >>> > >> >>> > >> >>> This doesn't always seem to happen on the same page, and it isn't > >> >>> always the same component that throws the exception. (Here it is > >> >>> PageLink, I've also seen Any and Body throw this). > >> >>> > >> >>> I've been scratching my head on this for a while, trying to figure > >> >>> out why we hadn't seen this in previous releases... Then I > >> >>> remembered we switch to 3.0.4, and I decided to look into the > >> >>> differences. > >> >>> > >> >>> I found something in the DefaultComponentClassEnhancer which has > >> >>> changed, and I'd be interested in hearing if anybody believes this > >> >>> could cause my problem. > >> >>> > >> >>> Specifically the method getEnhancedClass has changed from the > >> >>> following in 3.0.3 > >> >>> > >> >>> public Class getEnhancedClass(IComponentSpecification > specification, > >> >>> String className) > >> >>> { > >> >>> Class result = getCachedClass(specification); > >> >>> > >> >>> if (result == null) > >> >>> { > >> >>> synchronized(this) > >> >>> { > >> >>> result = getCachedClass(specification); > >> >>> if (result == null) > >> >>> { > >> >>> result = constructComponentClass(specification, > >> >>> className); > >> >>> storeCachedClass(specification, result); > >> >>> } > >> >>> } > >> >>> } > >> >>> return result; > >> >>> } > >> >>> > >> >>> to this in 3.0.4 > >> >>> > >> >>> public Class getEnhancedClass(IComponentSpecification > specification, > >> >>> String className) > >> >>> { > >> >>> synchronized(specification) > >> >>> { > >> >>> Class result = getCachedClass(specification); > >> >>> if (result == null) > >> >>> { > >> >>> result = constructComponentClass(specification, > >> className); > >> >>> storeCachedClass(specification, result); > >> >>> } > >> >>> return result; > >> >>> } > >> >>> } > >> >>> > >> >>> Now, my understanding of the tapestry internals has plenty of holes > >> >>> in it, so I don't know if this can be related to my problem or not, > >> >>> but the circumstances are such that it feels like a likely > candidate. > >> >>> > >> >>> To try to test this theory myself, I have created a custom > >> >>> ComponentClassEnhancer implementation which basically reverts back > >> >>> to the 3.0.3 synchronization style - however, I haven't had an > >> >>> opportunity to get it into a load test environment yet. I should be > >> >>> able to do this sometime early this week. > >> >>> > >> >>> I'd love to hear from any developers who feel they can verify that > >> >>> this is my problem, or debunk it totally. :) > >> >>> > >> >>> Thanks much, > >> >>> Jeff Poetker > >> >>> > >> >>> > >> >>> > --------------------------------------------------------------------- > >> >>> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> >>> For additional commands, e-mail: [EMAIL PROTECTED] > >> >>> > >> >>> > >> >> > >> >> > >> >> > --------------------------------------------------------------------- > >> >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> >> For additional commands, e-mail: [EMAIL PROTECTED] > >> >> > >> > > >> > > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> For additional commands, e-mail: [EMAIL PROTECTED] > >> > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > -- Jesse Kuhnert Tapestry/Dojo/(and a dash of TestNG), team member/developer Open source based consulting work centered around dojo/tapestry/tacos/hivemind. http://blog.opencomponentry.com
