-
1. Contention on MINA? WAS: Combined codec and packet
clebert.suconic May 9, 2008 10:05 AM (in response to timfox)Yesterday... Just because I wanted to have a little better understanding of the packages.. I navigated through the code with a debugger :-) And I had seen one interesting thing.
org.apache.mina.common.DefaultIoSessionDataStructureFactory has few synchronized collections:private static class DefaultIoSessionAttributeMap implements IoSessionAttributeMap { private final Map<Object, Object> attributes = Collections.synchronizedMap(new HashMap<Object, Object>(4));
And on a test I executed today, this appeared in few threads:pool-3-thread-2" prio=1 tid=0x00007f611431de00 nid=0x32aa waiting for monitor entry [0x000000004444b000..0x000000004444be20] at java.util.Collections$SynchronizedMap.get(Collections.java:1979) - waiting to lock <0x00007f61296c47c8> (a java.util.Collections$SynchronizedMap) at org.apache.mina.common.DefaultIoSessionDataStructureFactory$DefaultIoSessionAttributeMap.getAttribute(DefaultIoSessionDataStructureFactory.java:63) at org.apache.mina.common.AbstractIoSession.getAttribute(AbstractIoSession.java:343) at org.apache.mina.common.AbstractIoSession.getAttribute(AbstractIoSession.java:339) at org.apache.mina.common.SimpleIoProcessorPool.getProcessor(SimpleIoProcessorPool.java:236) at org.apache.mina.common.SimpleIoProcessorPool.flush(SimpleIoProcessorPool.java:181) at org.apache.mina.common.SimpleIoProcessorPool.flush(SimpleIoProcessorPool.java:1) at org.apache.mina.common.DefaultIoFilterChain$HeadFilter.filterWrite(DefaultIoFilterChain.java:644) at org.apache.mina.common.DefaultIoFilterChain.callPreviousFilterWrite(DefaultIoFilterChain.java:467) at org.apache.mina.common.DefaultIoFilterChain.access$7(DefaultIoFilterChain.java:464) at org.apache.mina.common.DefaultIoFilterChain$EntryImpl$1.filterWrite(DefaultIoFilterChain.java:835)
Shouldn't this be using concurrent?
Are we (including Trustin) aware of that specific possible contention? -
2. Re: Combined codec and packet
trustin May 9, 2008 10:38 AM (in response to timfox)Yes. The default attribute map is a synchronized hash map because concurrent hash map consumes more memory as the number of the connections grows. You can change the default map by implementing your own IoSessionDataStructureFactory and setting its new instance in IoAcceptor or IoConnector.
Hmm, or MINA could provide a concurrent hash map backed implementation out of the box (although it will not be the default).
Please let me know if we can decrease the memory consumption with concurrent hash map. That would be the best solution. -
3. Re: Combined codec and packet
clebert.suconic May 9, 2008 11:02 AM (in response to timfox)Yes. The default attribute map is a synchronized hash map because concurrent hash map consumes more memory as the number of the connections grows.
Is it really relevant? It will just provide partitioned almost empty HashMaps to avoid the concurrency on a single object.
And.. isn't this synchronization limiting the number of connection you can reach up to as it is a contention point? -
4. Re: Combined codec and packet
timfox May 9, 2008 11:04 AM (in response to timfox)I am trying to remove or at least minimise our usage of session attributes anyway so this may be moot.
-
5. Re: Combined codec and packet
clebert.suconic May 9, 2008 11:11 AM (in response to timfox)"timfox" wrote:
I am trying to remove or at least minimise our usage of session attributes anyway so this may be moot.
Well.. if we need to keep them.. we can just replace the IoSessionDataStructureFactory by a Concurrent implementation. -
6. Re: Combined codec and packet
trustin May 9, 2008 11:21 AM (in response to timfox)"clebert.suconic@jboss.com" wrote:
Yes. The default attribute map is a synchronized hash map because concurrent hash map consumes more memory as the number of the connections grows.
Is it really relevant? It will just provide partitioned almost empty HashMaps to avoid the concurrency on a single object.
And.. isn't this synchronization limiting the number of connection you can reach up to as it is a contention point?
Yes, the memory consumption matters when the number of connection becomes very large (e.g. 10000.) MINA team got complaints as soon as we switched to ConcurrentHashMap and that's why we switched back to synchronized HashMap. In most cases, the number of threads which accesses the synchronized map will be less than 3, so it is often OK to use a synchronized map. -
7. Re: Combined codec and packet
timfox May 9, 2008 12:44 PM (in response to timfox)"trustin" wrote:
Hmm, or MINA could provide a concurrent hash map backed implementation out of the box (although it will not be the default).
I like the idea of MINA having some config switch boolean "useConcurrentSessionMap" which if true uses a ConcurrentHashMap and if false uses the synchronized map. Then the user can choose which to use based on their number of expected connections and available RAM. This can default to false to preserve current behaviour. -
8. Re: Combined codec and packet
clebert.suconic May 9, 2008 1:32 PM (in response to timfox)"timfox" wrote:
"trustin" wrote:
Hmm, or MINA could provide a concurrent hash map backed implementation out of the box (although it will not be the default).
I like the idea of MINA having some config switch boolean "useConcurrentSessionMap" which if true uses a ConcurrentHashMap and if false uses the synchronized map. Then the user can choose which to use based on their number of expected connections and available RAM. This can default to false to preserve current behaviour.
+1 -
9. Re: Combined codec and packet
timfox May 13, 2008 12:57 PM (in response to timfox)For now I have replaced the default implementation with our own implementation based on a ConcurrentHashMap and this has alleviated the point of contention.