[ 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)