You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by bu...@apache.org on 2020/02/21 20:11:18 UTC

[cxf] 01/02: CXF-8149 Reduce synchronization in AbstractJXBProvider (#597)

This is an automated email from the ASF dual-hosted git repository.

buhhunyx pushed a commit to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 27d12fe0b183fff3ef5d507356b879c55856ca6b
Author: Alexey Markevich <bu...@gmail.com>
AuthorDate: Fri Feb 21 09:42:15 2020 +0300

    CXF-8149 Reduce synchronization in AbstractJXBProvider (#597)
    
    * CXF-8149 Reduce synchronization in AbstractJXBProvider
    
    * use javax.xml.ws.Holder; improve getCollectionContext impl
    
    * use single value array for possible exception propagation
    
    (cherry picked from commit 224fa741d6753053c6f47d361f8cc7e93fd34ed9)
---
 .../cxf/jaxrs/provider/AbstractJAXBProvider.java   | 115 ++++++++++-----------
 1 file changed, 55 insertions(+), 60 deletions(-)

diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/AbstractJAXBProvider.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/AbstractJAXBProvider.java
index 9704a91..a9500e5 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/AbstractJAXBProvider.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/AbstractJAXBProvider.java
@@ -32,12 +32,12 @@ import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.MediaType;
@@ -98,7 +98,7 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
         new HashSet<Class<?>>(Arrays.asList(InputStream.class,
                                             OutputStream.class,
                                             StreamingOutput.class));
-    protected Set<Class<?>> collectionContextClasses = new HashSet<>();
+    protected Set<Class<?>> collectionContextClasses = ConcurrentHashMap.newKeySet();
 
     protected Map<String, String> jaxbElementClassMap = Collections.emptyMap();
     protected boolean unmarshalAsJaxbElement;
@@ -111,8 +111,8 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
     protected List<String> inDropElements;
     protected Map<String, String> inElementsMap;
     protected Map<String, String> inAppendMap;
-    protected Map<String, JAXBContext> packageContexts = new HashMap<>();
-    protected Map<Class<?>, JAXBContext> classContexts = new HashMap<>();
+    protected Map<String, JAXBContext> packageContexts = new ConcurrentHashMap<>();
+    protected Map<Class<?>, JAXBContext> classContexts = new ConcurrentHashMap<>();
     private boolean attributesToElements;
 
     private MessageContext mc;
@@ -366,14 +366,11 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
     }
 
     protected JAXBContext getCollectionContext(Class<?> type) throws JAXBException {
-        synchronized (collectionContextClasses) {
-            if (!collectionContextClasses.contains(type)) {
-                collectionContextClasses.add(CollectionWrapper.class);
-                collectionContextClasses.add(type);
-            }
-            return newJAXBContextInstance(
-                collectionContextClasses.toArray(new Class[0]), cProperties);
+        if (collectionContextClasses.add(type)) {
+            collectionContextClasses.add(CollectionWrapper.class);
         }
+        return newJAXBContextInstance(
+            collectionContextClasses.toArray(new Class[0]), cProperties);
     }
 
     protected QName getCollectionWrapperQName(Class<?> cls, Type type, Object object, boolean pluralName)
@@ -506,14 +503,12 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
             }
         }
 
-        synchronized (classContexts) {
-            JAXBContext context = classContexts.get(type);
-            if (context != null) {
-                return context;
-            }
+        JAXBContext context = classContexts.get(type);
+        if (context != null) {
+            return context;
         }
 
-        JAXBContext context = getPackageContext(type, genericType);
+        context = getPackageContext(type, genericType);
 
         return context != null ? context : getClassContext(type, genericType);
     }
@@ -521,23 +516,28 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
         return getClassContext(type, type);
     }
     protected JAXBContext getClassContext(Class<?> type, Type genericType) throws JAXBException {
-        synchronized (classContexts) {
-            JAXBContext context = classContexts.get(type);
-            if (context == null) {
-                Class<?>[] classes;
-                if (extraClass != null) {
-                    classes = new Class[extraClass.length + 1];
-                    classes[0] = type;
-                    System.arraycopy(extraClass, 0, classes, 1, extraClass.length);
-                } else {
-                    classes = new Class[] {type};
-                }
+        final JAXBException[] jaxbException = new JAXBException[] {null};
+        final JAXBContext context = classContexts.computeIfAbsent(type, t -> {
+            final Class<?>[] classes;
+            if (extraClass != null) {
+                classes = new Class[extraClass.length + 1];
+                classes[0] = type;
+                System.arraycopy(extraClass, 0, classes, 1, extraClass.length);
+            } else {
+                classes = new Class[] {type};
+            }
 
-                context = newJAXBContextInstance(classes, cProperties);
-                classContexts.put(type, context);
+            try {
+                return newJAXBContextInstance(classes, cProperties);
+            } catch (JAXBException e) {
+                jaxbException[0] = e;
+                return null;
             }
-            return context;
+        });
+        if (null != jaxbException[0]) {
+            throw jaxbException[0];
         }
+        return context;
     }
     public JAXBContext getPackageContext(Class<?> type) {
         return getPackageContext(type, type);
@@ -546,40 +546,35 @@ public abstract class AbstractJAXBProvider<T> extends AbstractConfigurableProvid
         if (type == null || type == JAXBElement.class) {
             return null;
         }
-        synchronized (packageContexts) {
-            String packageName = PackageUtils.getPackageName(type);
-            JAXBContext context = packageContexts.get(packageName);
-            if (context == null) {
-                try {
-                    final ClassLoader loader = AccessController.doPrivileged((PrivilegedAction<ClassLoader>) 
-                        () -> {
-                            return type.getClassLoader();
-                        });
-                    if (loader != null && objectFactoryOrIndexAvailable(type)) {
-
-                        String contextName = packageName;
-                        if (extraClass != null) {
-                            StringBuilder sb = new StringBuilder(contextName);
-                            for (Class<?> extra : extraClass) {
-                                String extraPackage = PackageUtils.getPackageName(extra);
-                                if (!extraPackage.equals(packageName)) {
-                                    sb.append(':').append(extraPackage);
-                                }
+        final String packageName = PackageUtils.getPackageName(type);
+        return packageContexts.computeIfAbsent(packageName, p -> {
+            try {
+                final ClassLoader loader = AccessController.doPrivileged((PrivilegedAction<ClassLoader>) 
+                    () -> {
+                        return type.getClassLoader();
+                    });
+                if (loader != null && objectFactoryOrIndexAvailable(type)) {
+
+                    String contextName = packageName;
+                    if (extraClass != null) {
+                        StringBuilder sb = new StringBuilder(contextName);
+                        for (Class<?> extra : extraClass) {
+                            String extraPackage = PackageUtils.getPackageName(extra);
+                            if (!extraPackage.equals(packageName)) {
+                                sb.append(':').append(extraPackage);
                             }
-                            contextName = sb.toString();
                         }
-
-                        context = JAXBContext.newInstance(contextName, loader, cProperties);
-                        packageContexts.put(packageName, context);
+                        contextName = sb.toString();
                     }
-                } catch (JAXBException ex) {
-                    LOG.fine("Error creating a JAXBContext using ObjectFactory : "
-                                + ex.getMessage());
-                    return null;
+
+                    return JAXBContext.newInstance(contextName, loader, cProperties);
                 }
+            } catch (JAXBException ex) {
+                LOG.fine("Error creating a JAXBContext using ObjectFactory : "
+                            + ex.getMessage());
             }
-            return context;
-        }
+            return null;
+        });
     }
 
     protected boolean isSupported(Class<?> type, Type genericType, Annotation[] anns) {