-
Improvement
-
Resolution: Fixed
-
Critical
-
-
2.266
Related to JENKINS-13154, we are still seeing lots of congestion with Jenkins 1.517 and our builds may still stay tens of minutes in "Waiting for Jenkins to finish collecting data" -phase.
The place has changed though, now thread dumps show lots of these:
java.lang.Thread.State: BLOCKED (on object monitor)
at java.util.Collections$SynchronizedMap.get(Collections.java:2031)
waiting to lock <0x000000070c6dcfa8> (a java.util.Collections$SynchronizedMap)
at com.thoughtworks.xstream.core.DefaultConverterLookup.lookupConverterForType(DefaultConverterLookup.java:49)
- is blocking
-
JENKINS-46460 Performance issue in AsyncResourceDisposer.persist
-
- Resolved
-
- is related to
-
-
- Resolved
-
-
-
- Resolved
-
-
-
- Resolved
-
- relates to
-
-
- In Progress
-
- links to
(4 links to)
[JENKINS-19561] Unsafe & inefficient concurrency in XStream
Mikko Peltonen
created issue -
Mikko Peltonen
made changes -
| Link |
New:
This issue is related to |
| Labels | New: performance threads |
| Link |
New:
This issue is related to |
if CI uses XStream for a fixed set of classes
It is quite open-ended in fact.
Discussions in http://jira.codehaus.org/browse/XSTR-744 indicate that using a single XStream2 instance (actually there are a handful of such singletons, in Items, Run, UpdateCenter, and User, as well as the general one in Jenkins) is unsupported—since we have no idea which thread will use these instances, they should really be loaded from a ThreadLocal. Unfortunately these singletons are public fields, not gated through methods, and they are in fact stateful (plugins can add converters to them), so that is not straightforward.
Possibly XStream2 could be a façade which keeps a per-thread pool of actual implementations and delegates all method calls to them, which would allow us to remove the thread-related patches from the Jenkins fork, and which might permit better performance without sacrificing safety: there would be overhead for looking up the proxy, but this would only be incurred roughly once per XmlFile load, which is negligible compared to I/O costs. The rest of the load would only use monitors for the converter cache, which should be quick.
As of JENKINS-16301 in 1.518 fingerprint saving does not normally use XStream at all, so the issue is really about general (de)serialization performance.
| Summary | Original: Heavy thread congestion with FingerPrint.save sequel | New: Lock contention in XStream |
http://xstream.codehaus.org/annotations-tutorial.html#AutoDetect
Can also lead to deadlocks, such as this one loading lots of jobs:
Found one Java-level deadlock: ============================= "Loading job item-00006": waiting to lock monitor 0x00007f0df00054f8 (object 0x000000008a956ac8, a java.lang.Class), which is held by "Loading job item-00007" "Loading job item-00007": waiting to lock monitor 0x00007f0dfc3fe2f0 (object 0x000000008a955c08, a java.lang.Class), which is held by "Loading job item-00006" Java stack information for the threads listed above: =================================================== "Loading job item-00006": at java.lang.Class.initAnnotationsIfNecessary(Class.java:3095) - waiting to lock <0x000000008a956ac8> (a java.lang.Class for jenkins.model.lazy.LazyBuildMixIn$LazyLoadingJob) at java.lang.Class.getAnnotation(Class.java:3057) at com.thoughtworks.xstream.mapper.AnnotationMapper$UnprocessedTypesSet.add(AnnotationMapper.java:625) at com.thoughtworks.xstream.mapper.AnnotationMapper$UnprocessedTypesSet.add(AnnotationMapper.java:610) at com.thoughtworks.xstream.mapper.AnnotationMapper$1.add(AnnotationMapper.java:250) at com.thoughtworks.xstream.mapper.AnnotationMapper$1.add(AnnotationMapper.java:245) at com.thoughtworks.xstream.mapper.AnnotationMapper.addParametrizedTypes(AnnotationMapper.java:279) at com.thoughtworks.xstream.mapper.AnnotationMapper.processTypes(AnnotationMapper.java:204) - locked <0x000000008a955c08> (a java.lang.Class for jenkins.model.lazy.LazyBuildMixIn$LazyLoadingRun) at com.thoughtworks.xstream.mapper.AnnotationMapper.processAnnotations(AnnotationMapper.java:182) at com.thoughtworks.xstream.mapper.AnnotationMapper.defaultImplementationOf(AnnotationMapper.java:141) at hudson.util.xstream.MapperDelegate.defaultImplementationOf(MapperDelegate.java:59) at com.thoughtworks.xstream.mapper.MapperWrapper.defaultImplementationOf(MapperWrapper.java:46) at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:54) at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:50) at com.thoughtworks.xstream.core.TreeUnmarshaller.start(TreeUnmarshaller.java:134) at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.unmarshal(AbstractTreeMarshallingStrategy.java:32) at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1189) at hudson.util.XStream2.unmarshal(XStream2.java:113) at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1173) at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1053) at hudson.XmlFile.read(XmlFile.java:142) at hudson.model.Items.load(Items.java:277) at ... "Loading job item-00007": at java.lang.Class.initAnnotationsIfNecessary(Class.java:3095) - waiting to lock <0x000000008a955c08> (a java.lang.Class for jenkins.model.lazy.LazyBuildMixIn$LazyLoadingRun) at java.lang.Class.getAnnotation(Class.java:3057) at com.thoughtworks.xstream.mapper.AnnotationMapper$UnprocessedTypesSet.add(AnnotationMapper.java:625) at com.thoughtworks.xstream.mapper.AnnotationMapper$UnprocessedTypesSet.add(AnnotationMapper.java:610) at com.thoughtworks.xstream.mapper.AnnotationMapper$1.add(AnnotationMapper.java:250) at com.thoughtworks.xstream.mapper.AnnotationMapper$1.add(AnnotationMapper.java:245) at com.thoughtworks.xstream.mapper.AnnotationMapper.addParametrizedTypes(AnnotationMapper.java:279) at com.thoughtworks.xstream.mapper.AnnotationMapper.processTypes(AnnotationMapper.java:204) - locked <0x000000008a956ac8> (a java.lang.Class for jenkins.model.lazy.LazyBuildMixIn$LazyLoadingJob) at com.thoughtworks.xstream.mapper.AnnotationMapper.processAnnotations(AnnotationMapper.java:182) at com.thoughtworks.xstream.mapper.AnnotationMapper.getLocalConverter(AnnotationMapper.java:153) at hudson.util.xstream.MapperDelegate.getLocalConverter(MapperDelegate.java:112) at com.thoughtworks.xstream.mapper.MapperWrapper.getLocalConverter(MapperWrapper.java:99) at hudson.util.RobustReflectionConverter.unmarshalField(RobustReflectionConverter.java:348) at hudson.util.RobustReflectionConverter.doUnmarshal(RobustReflectionConverter.java:289) at hudson.util.RobustReflectionConverter.unmarshal(RobustReflectionConverter.java:228) at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:72) at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:65) at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:66) at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:50) at com.thoughtworks.xstream.core.TreeUnmarshaller.start(TreeUnmarshaller.java:134) at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.unmarshal(AbstractTreeMarshallingStrategy.java:32) at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1189) at hudson.util.XStream2.unmarshal(XStream2.java:113) at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1173) at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1053) at hudson.XmlFile.read(XmlFile.java:142) at hudson.model.Items.load(Items.java:277) at ...
| Priority | Original: Major [ 3 ] | New: Critical [ 2 ] |
| Summary | Original: Lock contention in XStream | New: Unsafe & inefficient concurrency in XStream |
Access to converters apparently needs to be synchronized per
JENKINS-18775but the execution path to the cached converters should be as free of synchronization as possible.typeToConverterMap has changed for more synchronization in two steps:
(1)
from ConcurrentHashMap to Collections.synchronizedMap(new WeakHashMap());
https://github.com/jenkinsci/xstream/blob/3ee1dba0e7a0ae2a12759c87bea9cbb6578e19f7/xstream/src/java/com/thoughtworks/xstream/core/DefaultConverterLookup.java
(2) from just having synchronized access to the map to total synchronization in
JENKINS-18775I assume the WeakHashMap is needed in XStream in the general case, but if CI uses XStream for a fixed set of classes we could maybe do without that so that ConcurrentHashMap would do the trick?