Hi Steven,
> Hi Adam, > > just some thoughts... > > > Very occasionally we have seen concurrent HashMap accesses causing 100% cpu usage in an endless tight loop. > > Does this refer to read, write or both types of access? >From memory, I believe it was concurrent read/write access that caused the 100% cpu issue, but my colleague that found the problem isn't around today so I can't confirm that. > > > ... we cannot know at initialisation time which class heirarchies will be required during the lifetime ... > > The test project provided by Simon generated the classes to be (un)marshalled with the Castor Source Generator. > So all classes should be known and all descriptors should be listed in .castor.cdr files. > So proper initialization at startup should be possible. > Or am I missing something? The test project was just that, a test project. Our webservice is deployed to the appservers as many seperate "handlers" each with their own Castor beans. Each handler may or may not get invoked during the lifetime of the application and it would be silly for us to pre-load all Castor beans at startup. We would much prefer the resolver to lazy-load the descriptors just in time to use them. > > > Patch Castor to use Hashtable instead of HashMap ... > > Most methods that change the contents of the maps in the XMLClassDescriptorResolverImpl.DescriptorCache are synchronized. > There are three methods missing that should be made synchronized for the XMLClassDescriptorResolver to be fully thread safe. > > Simply changing a Map (or Collection) to a synchronized one does *not* necessarily deliver the same results as making the method that modifies the Map (or Collection) synchronized. > I haven't, myself, looked at the Castor code base so I can't comment on any code specifics. I do understand that synchronizing the map isn't necessarily the only synchronization that would be required for this application of the Resolver. My point is that the performance hit due to synchronizing the map is, i'm pretty sure, totally irrelevent compared to the parsing or pretty-printing of the XML. > For example XMLClassDescriptorResolverImpl.DescriptorCache#getDescriptorList(String): > Using a synchronized Map (or a Hashtable) might actually return two different instances of List for the same xmlName if called concurrently. > Making the whole method synchronized guarantees that only one instance per xmlName is created. > > > I said before that I was not sure whether this is fully thread safe or not. It just wasn't on my mind when I created the descriptor cache. > > Given that proper initialization at startup is not possible I agree with you that there is a (however improbable) risk of running into concurrency problems. > > My preferred solution for this would be to make the three missing methods (namely addDescriptor(String className, XMLClassDescriptor descriptor), addMissingDescriptor(String className), getDescriptorList(String xmlName)) synchronized. IMO this should eliminate all possible risks (assuming that read accesses do not need synchronization). > I am happy with your proposed solution as I believe this would probably be safe (assuming the hashmap 100% cpu issue wasn't caused solely through concurrent reads). We will wait for your updates and then Simon will write some highly concurrent tests which we will execute on one of our T1000's. We'll let you know how we get on! > > Regards, > Steven > Thanks for your time. Cheers, Adam. > > -------- Original-Nachricht -------- > Datum: Fri, 16 Mar 2007 12:52:45 -0000 > Von: "Adam J Chesney" <[EMAIL PROTECTED]> > An: [email protected] > CC: > Betreff: Re: AW: AW: [castor-user] [XML] XMLClassDescriptorResolver XMLFieldDescriptorImpl cache possible memory leak > > > Hi Leander & Werner, > > > > I work with Simon here at Multicom and I just wanted to share with you our > > experience of HashMap's in a highly concurrent environment. I haven't ever > > attempted to replicate the following problem with a unit test and so it is > > only based on empirical evidence of running our web service under Java 5 > > on > > both linux (x86) and solaris (sun fire T1000 upto 32 simultaneous > > threads). > > > > Very occasionally we have seen concurrent HashMap accesses causing 100% > > cpu > > usage in an endless tight loop. It is exrememly rare (before we worked out > > what it was it might have happened once a or twice a month across 6 T1000 > > appservers), and I'm not sure exactly how it happens, but it does happen > > and > > it can bring our application servers to their knees. As such, we have a > > strict policy of checking HashMap usage in our own code here at Multicom > > and > > we have even had to produce our own patched versions of a few open source > > tools in order to remove concurrent HashMap usage. > > > > We are a massive user of Castor as it probably accounts for 20% to 30% of > > our total CPU usage (we pretty much act as an ecommerce market place > > connecting many different vendors to many different vendee's via an XML > > based hub) and so we would love to take advantage of ClassDescriptor > > caching > > that the Resolver provides. However, we are in a sitatuation where many > > different (>30) class heirarchies will require marshalling and > > unmarshalling > > (each vendor will have their own message set and their own Castor beans > > Jar) > > and we cannot know at initialisation time which class heirarchies will be > > required during the lifetime of the ClassDescriptorResolver. Therefore it > > is > > inevitable that the Maps holding the class descriptors may well continue > > to > > be updated throught the lifetime of the Resolver, and therefore as I see > > it > > we have 2 options: > > > > A) Implement a Thread safe pool of Resolvers (one pool per class heirarchy > > to save memory) - Simon has already done this and it works fine and will > > be > > safe to use. > > > > B) Patch Castor to use Hashtable instead of HashMap (or > > Collections.synchronizeMap (hashmap) if null values are required) > > > > The advantage of B) is reduced memory footprint and it works out of the > > box. > > As I see it, the very slight performence hit on using a synchronized Map > > instead of an unsynchronized one is vastly overshadowed by the inability > > of > > Castor to bring our servers to their knees. :-) > > > > So... what do you guys think? > > > > Cheers, > > > > Adam Chesney > > Senior Architect > > Multicom Products Ltd > > > > ===================================================== > > > > Tel: (+44) 117 908 1254 (Direct) > > Mobile (+44) 7780 962 961 > > email: [EMAIL PROTECTED] > > > > This communication is intended solely for the addressee and is > > confidential. > > If you are not the intended recipient, any disclosure, copying, > > distribution > > or any action taken or omitted to be taken in reliance on it, is > > prohibited > > and may be unlawful. > > > > Although this e-mail and any attachments are believed to be free of any > > virus, or any other defect which might affect any computer or IT system > > into > > which they are received and opened, it is the responsibility of the > > recipient to ensure that they are virus free and no responsibility is > > accepted by Multicom Products Limited for any loss or damage arising in > > any > > way from receipt or use thereof. > > > > ----- Original Message ----- > > From: "M.-Leander Reimer" <[EMAIL PROTECTED]> > > To: <[email protected]> > > Sent: Friday, March 16, 2007 11:25 AM > > Subject: Re: AW: AW: [castor-user] [XML] XMLClassDescriptorResolver > > XMLFieldDescriptorImpl cache possible memory leak > > > > > > > Hi Simon and Werner, > > > > > > after running Simon's tests and seeing the memory results I started to > > > write for my own test code using JUnitPerf to load test my stuff using > > > multiple concurrent threads with several iterations. > > > > > > Everything runs fine. The memory stays constant and also the time taken > > > to unmarshall/marshall an XML structure. No concurrency problems > > detected. > > > > > > HOWEVER, the main difference to Simon's code is, that I initialize the > > > XMLClassdescriptorResolver using a mapping and I still use Castor 1.0.5! > > > > > > > > > After switching to Castor 1.1, all of the tests that use IDs in the > > > mapping do not run anymore. (stack trace at the end of my mail) > > > > > > After looking at the release notes I saw that there have been changes to > > > the ID resolution. But I guess that's a different issue and I will > > > investigate this a little more. > > > > > > > > > Regards, > > > Leander > > > > > > > > > ValidationException: The following exception occured while validating > > > field: Locales of class: > > > de.adaptions.adgen.common.fonts.localemapping.LocaleMapping: The object > > > associated with IDREF > > > "[EMAIL PROTECTED]" of type class > > > de.adaptions.adgen.common.fonts.localemapping.Locale has no ID!; > > > - location of error: XPATH: /locales > > > The object associated with IDREF > > > "[EMAIL PROTECTED]" of type class > > > de.adaptions.adgen.common.fonts.localemapping.Locale has no ID! > > > at > > org.exolab.castor.xml.FieldValidator.validate(FieldValidator.java:270) > > > at > > > > > org.exolab.castor.xml.util.XMLClassDescriptorImpl.validate(XMLClassDescripto > > rImpl.java:931) > > > at org.exolab.castor.xml.Validator.validate(Validator.java:124) > > > at > > > > > org.exolab.castor.xml.UnmarshalHandler.endElement(UnmarshalHandler.java:913) > > > at > > > > > org.exolab.castor.xml.UnmarshalHandler.endElement(UnmarshalHandler.java:1108 > > ) > > > at org.apache.xerces.parsers.AbstractSAXParser.endElement(Unknown > > Source) > > > at > > > > > org.apache.xerces.impl.XMLDocumentFragmentScannerImpl.scanEndElement(Unknown > > > Source) > > > at > > > > > org.apache.xerces.impl.XMLDocumentFragmentScannerImpl$FragmentContentDispatc > > her.dispatch(Unknown > > > Source) > > > at > > > > > org.apache.xerces.impl.XMLDocumentFragmentScannerImpl.scanDocument(Unknown > > > Source) > > > at org.apache.xerces.parsers.XML11Configuration.parse(Unknown Source) > > > at org.apache.xerces.parsers.XML11Configuration.parse(Unknown Source) > > > at org.apache.xerces.parsers.XMLParser.parse(Unknown Source) > > > at org.apache.xerces.parsers.AbstractSAXParser.parse(Unknown Source) > > > at org.exolab.castor.xml.Unmarshaller.unmarshal(Unmarshaller.java:726) > > > at > > > > > de.adaptions.adgen.common.util.xml.LocaleMappingSerializer.deserialize(Local > > eMappingSerializer.java:133) > > > at > > > > > de.adaptions.adgen.common.util.xml.LocaleMappingSerializerTest.testPerforman > > ce(LocaleMappingSerializerTest.java:56) > > > at sun.reflect.GeneratedMethodAccessor177.invoke(Unknown Source) > > > at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) > > > at java.lang.reflect.Method.invoke(Unknown Source) > > > at junit.framework.TestCase.runTest(TestCase.java:154) > > > at junit.framework.TestCase.runBare(TestCase.java:127) > > > at junit.framework.TestResult$1.protect(TestResult.java:106) > > > at junit.framework.TestResult.runProtected(TestResult.java:124) > > > at junit.framework.TestResult.run(TestResult.java:109) > > > at junit.framework.TestCase.run(TestCase.java:118) > > > at junit.framework.TestSuite.runTest(TestSuite.java:208) > > > at junit.framework.TestSuite.run(TestSuite.java:203) > > > at com.clarkware.junitperf.TestFactory.run(TestFactory.java:83) > > > at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) > > > at junit.extensions.TestDecorator.run(TestDecorator.java:28) > > > at > > > > > com.clarkware.junitperf.TimedTest.runUntilTestCompletion(TimedTest.java:154) > > > at com.clarkware.junitperf.TimedTest.run(TimedTest.java:138) > > > at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) > > > at junit.extensions.TestDecorator.run(TestDecorator.java:28) > > > at junit.extensions.RepeatedTest.run(RepeatedTest.java:25) > > > at > > > > > com.clarkware.junitperf.ThreadedTest$TestRunner.run(ThreadedTest.java:75) > > > at java.lang.Thread.run(Unknown Source) > > > Caused by: ValidationException: The object associated with IDREF > > > "[EMAIL PROTECTED]" of type class > > > de.adaptions.adgen.common.fonts.localemapping.Locale has no ID!; > > > - location of error: XPATH: /locale > > > at > > > > > org.exolab.castor.xml.validators.IdRefValidator.validate(IdRefValidator.java > > :70) > > > at > > org.exolab.castor.xml.FieldValidator.validate(FieldValidator.java:207) > > > at > > > > > org.exolab.castor.xml.util.XMLClassDescriptorImpl.validate(XMLClassDescripto > > rImpl.java:938) > > > at org.exolab.castor.xml.Validator.validate(Validator.java:124) > > > at > > > > > org.exolab.castor.xml.FieldValidator.validateInstance(FieldValidator.java:31 > > 8) > > > at > > org.exolab.castor.xml.FieldValidator.validate(FieldValidator.java:248) > > > ... 36 more > > > > > > --------------------------------------------------------------------- > > > To unsubscribe from this list please visit: > > > > > > http://xircles.codehaus.org/manage_email > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe from this list please visit: > > > > http://xircles.codehaus.org/manage_email > > -- > Ist Ihr Browser Vista-kompatibel? Jetzt die neuesten > Browser-Versionen downloaden: http://www.gmx.net/de/go/browser > > --------------------------------------------------------------------- > To unsubscribe from this list please visit: > > http://xircles.codehaus.org/manage_email > --------------------------------------------------------------------- To unsubscribe from this list please visit: http://xircles.codehaus.org/manage_email

