[ 
https://issues.apache.org/jira/browse/HADOOP-12475?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14956660#comment-14956660
 ] 

Duo Zhang commented on HADOOP-12475:
------------------------------------

{code:title=Client.java}
1483          connection = connections.get(remoteId);
1484          if (connection == null) {
1485            connection = new Connection(remoteId, serviceClass);
1486            connections.put(remoteId, connection);
1487          }
{code}

I think the behavior here is not same with original code? The guava cache will 
prevent creating multiple connections for the same remoteId, but the CHM 
version will not and I think there will be connection leak...

> Replace guava Cache with ConcurrentHashMap for caching Connection in ipc 
> Client
> -------------------------------------------------------------------------------
>
>                 Key: HADOOP-12475
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12475
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: conf, io, ipc
>            Reporter: Walter Su
>            Assignee: Walter Su
>         Attachments: HADOOP-12475.01.patch
>
>
> quote [~daryn] from HADOOP-11772:
> {quote}
> CacheBuilder is obscenely expensive for concurrent map, and it requires 
> generating unnecessary garbage even just to look up a key. Replace it with 
> ConcurrentHashMap.
> I identified this issue that impaired my own perf testing under load. The 
> slowdown isn't just the sync. It's the expensive of Connection's ctor 
> stalling other connections. The expensive of ConnectionId#equals causes 
> delays. Synch'ing on connections causes unfair contention unlike a sync'ed 
> method. Concurrency simply hides this.
> {quote}
> BTW, guava Cache is heavyweight. Per local test, ConcurrentHashMap has better 
> overal performance.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to