From 6e783d11af4f8d9692f84cf63a46e8fd146a41fc Mon Sep 17 00:00:00 2001 From: Dinesh-Khandelwal Date: Sat, 20 Aug 2016 09:52:37 +0530 Subject: [PATCH 1/3] Performance-fixes Performance-fixes --- .../castor/mapping/MappingUnmarshaller.java | 10 ++ .../exolab/castor/xml/CastorThreadLocal.java | 52 ++++++++ .../org/exolab/castor/xml/XMLContext.java | 19 ++- .../util/XMLClassDescriptorResolverImpl.java | 111 ++++++++++++------ .../xml/util/resolvers/ByDescriptorClass.java | 32 +++-- .../xml/util/resolvers/CastorXMLStrategy.java | 8 +- 6 files changed, 172 insertions(+), 60 deletions(-) create mode 100644 xml/src/main/java/org/exolab/castor/xml/CastorThreadLocal.java diff --git a/xml/src/main/java/org/castor/mapping/MappingUnmarshaller.java b/xml/src/main/java/org/castor/mapping/MappingUnmarshaller.java index 3571b382b..a00aa8cd4 100644 --- a/xml/src/main/java/org/castor/mapping/MappingUnmarshaller.java +++ b/xml/src/main/java/org/castor/mapping/MappingUnmarshaller.java @@ -102,6 +102,16 @@ public MappingUnmarshaller() { _internalContext = internalContext; } + + /** + * This API is created to use the existing XMLContext object instead of creating a new one. + * @param internalContext + */ + public MappingUnmarshaller(InternalContext internalContext) { + _registry = new MappingLoaderRegistry(new CoreProperties()); + _idResolver = new MappingUnmarshallIDResolver(); + _internalContext = internalContext; + } /** * Enables or disables the ability to allow the redefinition of class mappings. diff --git a/xml/src/main/java/org/exolab/castor/xml/CastorThreadLocal.java b/xml/src/main/java/org/exolab/castor/xml/CastorThreadLocal.java new file mode 100644 index 000000000..c3b3dd616 --- /dev/null +++ b/xml/src/main/java/org/exolab/castor/xml/CastorThreadLocal.java @@ -0,0 +1,52 @@ +/** + * This class is created to support ThreadLocal in Castor. This class is introduced to support + * different xml document requests SOAP/JMS etc. having the same xml name as root element. If xml names are same with + * different incoming xml requests causes issues with Castor when Castor caching is used as Castor only expecting + * one-to-one mapping between Xml element and corresponding ComplexType. The required context is set by the application + * as required to set in the ThreadLocal. + * mappingInfo.put("class", "com.org.Customer"); + * mappingInfo.put("map-to", "cust"); + * mappingInfo.put("class", "com.org.SpecialCustomer"); + * mappingInfo.put("map-to", "cust"); + * + */ +package org.exolab.castor.xml; + +import java.util.WeakHashMap; + +/** + * This class provides thread-local variables. These + * variables differ from their normal counterparts in that each thread that + * accesses one (via its get or set method) has its own, independently + * initialised copy of the variable. + * + */ +public class CastorThreadLocal { + private static ThreadLocal> threadLocal = new ThreadLocal>(); + private static final String REQ_CONTEXT = "REQ_CONTEXT"; + + private static WeakHashMap getDetailsMap() { + WeakHashMap detailsMap = threadLocal.get(); + if (detailsMap == null) { + detailsMap = new WeakHashMap(); + threadLocal.set(detailsMap); + } + return detailsMap; + } + + /** + * This request Context is MF ID. Appending this to the xml element will make the element + * unique across web services calls. + * @return + */ + public static String getReqContext() { + if (getDetailsMap().get(REQ_CONTEXT) != null) { + return (String) getDetailsMap().get(REQ_CONTEXT); + } + return null; + } + + public static void setReqContext(String reqContext) { + getDetailsMap().put(REQ_CONTEXT, reqContext); + } +} diff --git a/xml/src/main/java/org/exolab/castor/xml/XMLContext.java b/xml/src/main/java/org/exolab/castor/xml/XMLContext.java index 55171915c..e281334ea 100644 --- a/xml/src/main/java/org/exolab/castor/xml/XMLContext.java +++ b/xml/src/main/java/org/exolab/castor/xml/XMLContext.java @@ -70,17 +70,18 @@ public XMLContext() { } /** - * Instructs Castor to load class descriptors from the mapping given. - * + * Instructs Castor to load class descriptors from the mapping given. Here the same _internalContext + * object is passed to MappingUnmarshaller so that it makes use of the same descriptor caching of the + * _internalContext object. * @param mapping Castor XML mapping (file), from which the required class descriptors will be * derived. * @throws MappingException If the {@link Mapping} cannot be loaded and analyzed successfully. */ public void addMapping(final Mapping mapping) throws MappingException { - MappingUnmarshaller mappingUnmarshaller = new MappingUnmarshaller(); - MappingLoader mappingLoader = mappingUnmarshaller.getMappingLoader(mapping, BindingType.XML); - _internalContext.getXMLClassDescriptorResolver().setMappingLoader(mappingLoader); - } + MappingUnmarshaller mappingUnmarshaller = new MappingUnmarshaller(_internalContext); + MappingLoader mappingLoader = mappingUnmarshaller.getMappingLoader(mapping, BindingType.XML); + _internalContext.getXMLClassDescriptorResolver().setMappingLoader(mappingLoader); + } /** * Loads the class descriptor for the class instance specified. The use of this method is useful @@ -287,4 +288,10 @@ public InternalContext getInternalContext() { public void setClassLoader(ClassLoader classLoader) { this._internalContext.setClassLoader(classLoader); } + /** + * This is called by the application to set the right context. + */ + public void setReqContext(String requestContext) { + CastorThreadLocal.setReqContext(requestContext); + } } diff --git a/xml/src/main/java/org/exolab/castor/xml/util/XMLClassDescriptorResolverImpl.java b/xml/src/main/java/org/exolab/castor/xml/util/XMLClassDescriptorResolverImpl.java index c40d3afea..249d83d83 100644 --- a/xml/src/main/java/org/exolab/castor/xml/util/XMLClassDescriptorResolverImpl.java +++ b/xml/src/main/java/org/exolab/castor/xml/util/XMLClassDescriptorResolverImpl.java @@ -50,6 +50,7 @@ import org.castor.xml.InternalContext; import org.exolab.castor.mapping.ClassDescriptor; import org.exolab.castor.mapping.MappingLoader; +import org.exolab.castor.xml.CastorThreadLocal; import org.exolab.castor.xml.Introspector; import org.exolab.castor.xml.ResolverException; import org.exolab.castor.xml.XMLClassDescriptor; @@ -153,7 +154,7 @@ public void setLoadPackageMappings(final boolean loadPackageMappings) { * {@inheritDoc} */ public void setMappingLoader(final MappingLoader mappingLoader) { - _mappingLoader = mappingLoader; + // _mappingLoader = mappingLoader; if (mappingLoader != null) { for (ClassDescriptor classDescriptor : mappingLoader.getDescriptors()) { _descriptorCache.addDescriptor(classDescriptor.getJavaClass().getName(), @@ -303,25 +304,29 @@ public XMLClassDescriptor resolveByXMLName(final String xmlName, final String na // nothing matches that XML name return null; } - if (possibleMatches.size() == 1) { - // we have exactly one possible match - that's our result - // (if it has the right namespace, it's an exact match, if not its - // the only possible match) - return (XMLClassDescriptor) possibleMatches.get(0); - } - - // we have more than one result - only an exact match can be the result - for (Iterator i = possibleMatches.iterator(); i.hasNext();) { - XMLClassDescriptor descriptor = (XMLClassDescriptor) i.next(); - - if (ResolveHelpers.namespaceEquals(namespaceURI, descriptor.getNameSpaceURI())) { - return descriptor; - } + if (possibleMatches.size() == 1) { + // we have exactly one possible match - that's our result + // (if it has the right namespace, it's an exact match, if not its + // the only possible match) + return (XMLClassDescriptor) possibleMatches.get(0); + } + // we have more than one result - only an exact match can be the result + return findExactMatch(namespaceURI, possibleMatches); + // no exact match and too many possible matches... + // return null; + } //-- resolveByXMLName + + private XMLClassDescriptor findExactMatch(final String namespaceURI, List possibleMatches) { + for (Iterator i = possibleMatches.iterator(); i.hasNext();) { + XMLClassDescriptor descriptor = (XMLClassDescriptor) i.next(); + if (ResolveHelpers.namespaceEquals(namespaceURI, descriptor.getNameSpaceURI())) { + return descriptor; + } } // no exact match and too many possible matches... - return null; - } // -- resolveByXMLName + return null; + } /** * {@inheritDoc} @@ -448,6 +453,7 @@ private static class DescriptorCacheImpl implements ResolverStrategy.ResolverRes /** Lock used to isolate write accesses to the caches internal lists and maps. */ private final ReentrantReadWriteLock _lock; + private final String SEPERATOR = "¤"; /** * Default constructor.
@@ -515,16 +521,21 @@ public void addDescriptor(final String className, final XMLClassDescriptor descr if (INTERNAL_CONTAINER_NAME.equals(xmlName)) { return; } - + // The below condition is to check if the input class is actually a xml element with out namespace. + if(descriptor instanceof XMLClassDescriptorAdapter && descriptor.getNameSpaceURI() == null){ + xmlName = getModifiedXmlElementName(xmlName); + } // add new descriptor to the list for the corresponding XML name List descriptorList = _xmlNameMap.get(xmlName); if (descriptorList == null) { descriptorList = new ArrayList(); _xmlNameMap.put(xmlName, descriptorList); } - if (!descriptorList.contains(descriptor)) { - descriptorList.add(descriptor); - } + // change by Dinesh K. Dont use .contains instead check with Name Space URI as .contains creates + // memory leak if some how the .CDR file is not proper and the descriptors are loaded again. + if (findExactMatch(descriptor.getNameSpaceURI(), descriptorList) == null) { + descriptorList.add(descriptor); + } _missingTypes.remove(className); } finally { @@ -532,6 +543,24 @@ public void addDescriptor(final String className, final XMLClassDescriptor descr } } // -- addDescriptor + private String getModifiedXmlElementName(final String className) { + String orgClassName = className; + if (CastorThreadLocal.getReqContext() != null && CastorThreadLocal.getReqContext() != "") { + // To check , element should not have "." as part of their name. + orgClassName = className + SEPERATOR + CastorThreadLocal.getReqContext(); + } + return orgClassName; + } + + private XMLClassDescriptor findExactMatch(final String namespaceURI, List possibleMatches) { + for (Iterator i = possibleMatches.iterator(); i.hasNext();) { + XMLClassDescriptor descriptor = (XMLClassDescriptor) i.next(); + if (ResolveHelpers.namespaceEquals(namespaceURI, descriptor.getNameSpaceURI())) { + return descriptor; + } + } + return null; + } /** * Gets the descriptor that is mapped to the given class name. * @@ -569,24 +598,28 @@ public XMLClassDescriptor getDescriptor(final String className) { * @return A list of descriptors with the given XML name or an empty list if no such descriptor * is stored in this cache. This method will never return null! */ - public List getDescriptors(final String xmlName) { - - // before accessing XML name map acquire read lock first - _lock.readLock().lock(); - List list = _xmlNameMap.get(xmlName); - _lock.readLock().unlock(); - - if (list == null) { - - // return an empty list - list = new ArrayList(); - } else { - - // return a copy of the original list - list = new ArrayList(list); - } - return list; - } // -- getDescriptorList + public List getDescriptors(final String xmlName) { + // before accessing XML name map acquire read lock first + _lock.readLock().lock(); + List list = null; + // Check first with modified xml name. + String modifiedXmlName = getModifiedXmlElementName(xmlName); + list = _xmlNameMap.get(modifiedXmlName); + if(list == null || (list !=null && list.size() ==0)){ + // we should try with actual xml name now. + list = _xmlNameMap.get(xmlName); + } + _lock.readLock().unlock(); + if (list == null) { + // return an empty list + list = new ArrayList(); + } + else { + // return a copy of the original list + list = new ArrayList(list); + } + return list; + } //-- getDescriptorList /** * Checks whether the given class name is contained in the list of class names the descriptor is diff --git a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/ByDescriptorClass.java b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/ByDescriptorClass.java index 00a9e6100..26734022f 100644 --- a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/ByDescriptorClass.java +++ b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/ByDescriptorClass.java @@ -56,19 +56,25 @@ protected Map internalResolve(final String className, final ClassLoader classLoa return results; } - StringBuilder descriptorClassName = new StringBuilder(className); - descriptorClassName.append(XMLConstants.DESCRIPTOR_SUFFIX); - Class descriptorClass = ResolveHelpers.loadClass(classLoader, descriptorClassName.toString()); - - // If we didn't find the descriptor, look in descriptor package - if (descriptorClass == null) { - int offset = descriptorClassName.lastIndexOf("."); - if (offset != -1) { - descriptorClassName.insert(offset, "."); - descriptorClassName.insert(offset + 1, XMLConstants.DESCRIPTOR_PACKAGE); - descriptorClass = ResolveHelpers.loadClass(classLoader, descriptorClassName.toString()); - } - } + Class descriptorClass = null; + StringBuilder descriptorClassName = new StringBuilder(className); + descriptorClassName.append(XMLConstants.DESCRIPTOR_SUFFIX); + // Try to load the below pattern first as it avoids searching for a descriptors which really does not exist which will help avoiding thread blocks on the //classloader for a wrong file pattern. + int offset = descriptorClassName.lastIndexOf("."); + if (offset != -1) { + descriptorClassName.insert(offset , "."); + descriptorClassName.insert(offset + 1, XMLConstants.DESCRIPTOR_PACKAGE); + descriptorClass = ResolveHelpers.loadClass( + classLoader, descriptorClassName.toString()); + } + + // Try with Castor default pattern. + if (descriptorClass == null) { + StringBuilder defaultDescriptorClassName = new StringBuilder(className); + defaultDescriptorClassName.append(XMLConstants.DESCRIPTOR_SUFFIX); + descriptorClass = ResolveHelpers.loadClass( + classLoader, defaultDescriptorClassName.toString()); + } if (descriptorClass != null) { try { diff --git a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java index 4372d88d7..b95caae28 100644 --- a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java +++ b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java @@ -119,13 +119,17 @@ private XMLClassDescriptor getDescriptor(final ResolverStrategy.ResolverResults return descriptor; } - this.resolvePackage(resolverResults, packageName); + /** + * if any of the Descriptor entry mapping is missing , It tries to load the whole CDR file and creates + * happens on for that .cdr file which comes first in the class path. + */ + resolverResults.addAllDescriptors(new ByDescriptorClass().resolve(className, _properties)); descriptor = resolverResults.getDescriptor(className); if (descriptor != null) { return descriptor; } - resolverResults.addAllDescriptors(new ByDescriptorClass().resolve(className, _properties)); + this.resolvePackage(resolverResults, packageName); descriptor = resolverResults.getDescriptor(className); if (descriptor != null) { return descriptor; From da6ac125d65e8fa0a5e1df1852420dd42aaf4fc4 Mon Sep 17 00:00:00 2001 From: Dinesh-Khandelwal Date: Sat, 20 Aug 2016 09:52:55 +0530 Subject: [PATCH 2/3] Performance-fixes --- .../exolab/castor/xml/util/resolvers/CastorXMLStrategy.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java index b95caae28..c5e1ac174 100644 --- a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java +++ b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java @@ -120,7 +120,11 @@ private XMLClassDescriptor getDescriptor(final ResolverStrategy.ResolverResults } /** + * Moving the search BYCDR later. First load by ByDescriptorClass using the right pattern. The issue with searching with CDR is, * if any of the Descriptor entry mapping is missing , It tries to load the whole CDR file and creates + * the instances of all the descriptors again and add it to cache , in caching as we check with .contais + * which fails as its a new instance of descriptors so it add it again and causes the memory leak. + * The issue is more observed when the .CDR files are maintained as part of different jars and loading * happens on for that .cdr file which comes first in the class path. */ resolverResults.addAllDescriptors(new ByDescriptorClass().resolve(className, _properties)); From 6d3589ba4ee8c43f2c45c0663684a9bc3b0679d9 Mon Sep 17 00:00:00 2001 From: Dinesh-Khandelwal Date: Sat, 20 Aug 2016 17:14:04 +0530 Subject: [PATCH 3/3] Performance-fixes Performance-fixes --- .../org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java index c5e1ac174..32cbdbbd5 100644 --- a/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java +++ b/xml/src/main/java/org/exolab/castor/xml/util/resolvers/CastorXMLStrategy.java @@ -122,7 +122,7 @@ private XMLClassDescriptor getDescriptor(final ResolverStrategy.ResolverResults /** * Moving the search BYCDR later. First load by ByDescriptorClass using the right pattern. The issue with searching with CDR is, * if any of the Descriptor entry mapping is missing , It tries to load the whole CDR file and creates - * the instances of all the descriptors again and add it to cache , in caching as we check with .contais + * the instances of all the descriptors again and add it to cache , in caching as we check with ".contais" * which fails as its a new instance of descriptors so it add it again and causes the memory leak. * The issue is more observed when the .CDR files are maintained as part of different jars and loading * happens on for that .cdr file which comes first in the class path.