Skip to content

Commit 1bde8d4

Browse files
committed
Avoid contention in DefaultConverterLookup by dropping synchronisation (closes jenkinsci#2).
1 parent c66e3ab commit 1bde8d4

File tree

1 file changed

+30
-18
lines changed

1 file changed

+30
-18
lines changed

xstream/src/java/com/thoughtworks/xstream/core/DefaultConverterLookup.java

+30-18
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,15 @@
1111
*/
1212
package com.thoughtworks.xstream.core;
1313

14-
import java.util.Collections;
15-
import java.util.Iterator;
14+
import java.util.HashMap;
1615
import java.util.LinkedHashMap;
1716
import java.util.Map;
18-
import java.util.WeakHashMap;
1917

2018
import com.thoughtworks.xstream.converters.ConversionException;
2119
import com.thoughtworks.xstream.converters.Converter;
2220
import com.thoughtworks.xstream.converters.ConverterLookup;
2321
import com.thoughtworks.xstream.converters.ConverterRegistry;
22+
import com.thoughtworks.xstream.core.util.Cloneables;
2423
import com.thoughtworks.xstream.core.util.PrioritizedList;
2524

2625

@@ -34,15 +33,28 @@
3433
public class DefaultConverterLookup implements ConverterLookup, ConverterRegistry, Caching {
3534

3635
private final PrioritizedList<Converter> converters = new PrioritizedList<>();
37-
private transient Map<Class<?>, Converter> typeToConverterMap;
36+
private transient Map<String, Converter> typeToConverterMap;
37+
private Map<String, Converter> serializationMap = null;
3838

3939
public DefaultConverterLookup() {
40-
readResolve();
40+
this(new HashMap<>());
41+
}
42+
43+
/**
44+
* Constructs a DefaultConverterLookup with a provided map.
45+
*
46+
* @param map the map to use
47+
* @throws NullPointerException if map is null
48+
* @since upcoming
49+
*/
50+
public DefaultConverterLookup(final Map<String, Converter> map) {
51+
typeToConverterMap = map;
52+
typeToConverterMap.clear();
4153
}
4254

4355
@Override
4456
public Converter lookupConverterForType(final Class<?> type) {
45-
final Converter cachedConverter = typeToConverterMap.get(type);
57+
final Converter cachedConverter = type != null ? typeToConverterMap.get(type.getName()) : null;
4658
if (cachedConverter != null) {
4759
return cachedConverter;
4860
}
@@ -51,6 +63,9 @@ public Converter lookupConverterForType(final Class<?> type) {
5163
for (final Converter converter : converters) {
5264
try {
5365
if (converter.canConvert(type)) {
66+
if (type != null) {
67+
typeToConverterMap.put(type.getName(), converter);
68+
}
5469
return converter;
5570
}
5671
} catch (final RuntimeException | LinkageError e) {
@@ -71,17 +86,8 @@ public Converter lookupConverterForType(final Class<?> type) {
7186

7287
@Override
7388
public void registerConverter(final Converter converter, final int priority) {
89+
typeToConverterMap.clear();
7490
converters.add(converter, priority);
75-
for (final Iterator<Class<?>> iter = typeToConverterMap.keySet().iterator(); iter.hasNext();) {
76-
final Class<?> type = iter.next();
77-
try {
78-
if (converter.canConvert(type)) {
79-
iter.remove();
80-
}
81-
} catch (final RuntimeException | LinkageError e) {
82-
// ignore
83-
}
84-
}
8591
}
8692

8793
@Override
@@ -94,9 +100,15 @@ public void flushCache() {
94100
}
95101
}
96102

103+
private Object writeReplace() {
104+
serializationMap = Cloneables.cloneIfPossible(typeToConverterMap);
105+
serializationMap.clear();
106+
return this;
107+
}
108+
97109
private Object readResolve() {
98-
// TODO: Use ConcurrentMap
99-
typeToConverterMap = Collections.synchronizedMap(new WeakHashMap<Class<?>, Converter>());
110+
typeToConverterMap = serializationMap == null ? new HashMap<>() : serializationMap;
111+
serializationMap = null;
100112
return this;
101113
}
102114
}

0 commit comments

Comments
 (0)