[
https://issues.apache.org/jira/browse/HADOOP-11659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14354265#comment-14354265
]
Akira AJISAKA commented on HADOOP-11659:
----------------------------------------
Hi [~brahmareddy], thanks for the patch.
The patch will change the behavior if {{map.contains(key)}} and {{fs !=
map.get(key)}}. I'm thinking the method still needs double lookups as follows:
{code}
synchronized void remove(Key key, FileSystem fs) {
assert fs != null;
if (map.get(key) == fs) {
map.remove(key);
toAutoClose.remove(key);
}
}
{code}
By the way, the method has two callers. One caller
{code:title=FileSystem.Cache#closeAll}
//remove from cache
remove(key, fs);
{code}
can be changed as
{code}
//remove from cache
map.remove(key);
toAutoClose.remove(key);
{code}
since {{map.contains(key)}} and {{fs == map.get(key)}} are guaranteed there.
That way we can remove the key from the map by single lookup.
> o.a.h.fs.FileSystem.Cache#remove should use a single hash map lookup
> --------------------------------------------------------------------
>
> Key: HADOOP-11659
> URL: https://issues.apache.org/jira/browse/HADOOP-11659
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 2.6.0
> Reporter: Gera Shegalov
> Assignee: Brahma Reddy Battula
> Priority: Minor
> Attachments: HADOOP-11659.patch
>
>
> The method looks up the same key in the same hash map potentially 3 times
> {code}
> if (map.containsKey(key) && fs == map.get(key)) {
> map.remove(key)
> {code}
> Instead it could do a single lookup
> {code}
> FileSystem cachedFs = map.remove(key);
> {code}
> and then test cachedFs == fs or something else.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
