[jira] [Resolved] (HADOOP-19346) ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent()

["Akira Ajisaka (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HADOOP-19346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Akira Ajisaka resolved HADOOP-19346.
------------------------------------
       Fix Version/s: 3.5.0
    Target Version/s:   (was: 3.5.1)
          Resolution: Fixed

The PR was already merged.

> ViewFileSystem.InnerCache: Replaced ReentrantReadWriteLock with ConcurrentHashMap/putIfAbsent()
> -----------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-19346
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19346
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: common
>            Reporter: Xing Lin
>            Assignee: Xing Lin
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 3.5.0
>
>
> Use of ReentrantReadWriteLock  + Map can be replaced/simplified with a ConcurrentHashMap. 
> Right now, we are using a ReentrantReadWriteLock to protect concurrent updates to InnerCache.map(). 
> As a performance optimization, to reduce acquiring writeLock, we first acquire readLock, check existence of the key and then acquire writeLock, to insert the new key.
> {code:java}
>     FileSystem get(URI uri, Configuration config) throws IOException {
>       Key key = new Key(uri);
>       FileSystem fs = null;
>       try {
>         rwLock.readLock().lock();
>         fs = map.get(key);
>         if (fs != null) {
>           return fs;
>         }
>       } finally {
>         rwLock.readLock().unlock();
>       }
>       try {
>         rwLock.writeLock().lock();
>         fs = map.get(key);
>         if (fs != null) {
>           return fs;
>         }
>         fs = fsCreator.getNewInstance(uri, config);
>         map.put(key, fs);
>         return fs;
>       } finally {
>         rwLock.writeLock().unlock();
>       }
>     }
> {code}
> The above function is already available as computeIfAbsent() from ConcurrentHashMap, which atomically checks existence of a key and inserts the new key if not there. This greatly simplifies the code. On the other hand, it should improve performance as well: concurrentHashMap supports finer-grain concurrency vs. a single ReadWrite Lock. 
> We can replace the above code with the following one and it is much simpler.
> {code:java}
>     FileSystem get(URI uri, Configuration config) throws IOException {
>       Key key = new Key(uri);
>       FileSystem fs = map.computeIfAbsent(key, k -> getNewFileSystem(uri, config));
>       if (fs == null) {
>         throw new IOException("Failed to create new FileSystem instance for " + uri);
>       }
>       return fs;
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org