Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance fixes #61

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions xml/src/main/java/org/castor/mapping/MappingUnmarshaller.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
52 changes: 52 additions & 0 deletions xml/src/main/java/org/exolab/castor/xml/CastorThreadLocal.java
Original file line number Diff line number Diff line change
@@ -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<WeakHashMap<String, Object>> threadLocal = new ThreadLocal<WeakHashMap<String, Object>>();
private static final String REQ_CONTEXT = "REQ_CONTEXT";

private static WeakHashMap<String, Object> getDetailsMap() {
WeakHashMap<String, Object> detailsMap = threadLocal.get();
if (detailsMap == null) {
detailsMap = new WeakHashMap<String, Object>();
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);
}
}
19 changes: 13 additions & 6 deletions xml/src/main/java/org/exolab/castor/xml/XMLContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<ClassDescriptor> 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<ClassDescriptor> possibleMatches) {
for (Iterator<ClassDescriptor> 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}
Expand Down Expand Up @@ -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.<br>
Expand Down Expand Up @@ -515,23 +521,46 @@ 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<ClassDescriptor> descriptorList = _xmlNameMap.get(xmlName);
if (descriptorList == null) {
descriptorList = new ArrayList<ClassDescriptor>();
_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 {
_lock.writeLock().unlock();
}
} // -- 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<ClassDescriptor> possibleMatches) {
for (Iterator<ClassDescriptor> 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.
*
Expand Down Expand Up @@ -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 <code>null</code>!
*/
public List<ClassDescriptor> getDescriptors(final String xmlName) {

// before accessing XML name map acquire read lock first
_lock.readLock().lock();
List<ClassDescriptor> list = _xmlNameMap.get(xmlName);
_lock.readLock().unlock();

if (list == null) {

// return an empty list
list = new ArrayList<ClassDescriptor>();
} else {

// return a copy of the original list
list = new ArrayList<ClassDescriptor>(list);
}
return list;
} // -- getDescriptorList
public List<ClassDescriptor> getDescriptors(final String xmlName) {
// before accessing XML name map acquire read lock first
_lock.readLock().lock();
List<ClassDescriptor> 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<ClassDescriptor>();
}
else {
// return a copy of the original list
list = new ArrayList<ClassDescriptor>(list);
}
return list;
} //-- getDescriptorList

/**
* Checks whether the given class name is contained in the list of class names the descriptor is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,21 @@ private XMLClassDescriptor getDescriptor(final ResolverStrategy.ResolverResults
return descriptor;
}

this.resolvePackage(resolverResults, packageName);
/**
* 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));
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;
Expand Down