lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <j...@apache.org>
Subject [jira] Commented: (LUCENE-2779) Use ConcurrentHashMap in RAMDirectory
Date Tue, 30 Nov 2010 19:24:10 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-2779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12965359#action_12965359
] 

Shai Erera commented on LUCENE-2779:
------------------------------------

I did hit an AIOOBE though, and I use IBM's JDK, which its CHM apparently uses AbstractCollection's
impl of toArray(Object[]). The question is, why should we rely on toArray() implemented one
way or the other in which JDK? If we think the best impl would be to allocate an ArrayList
and add all the elements to it, then why not do this explicitly? Remember that before this
change, listAll() would do almost exactly that - it iterated on the keySet() and added to
items to an array, only then it could rely on size().

Apparently, doing new ArrayList(fileMap.keySet()) is not safe either, as internally it allocates
an array and calls the set's toArray. So I ended up writing the following code and comment:

{code}
    // NOTE: due to different implementations of different JDKs, it's not safe
    // to do either:
    // 1. return fileMap.keySet().toArray(new String[0])
    // 2. return new ArrayList<String>(fileMap.keySet()).toArray(new String[0])
    // as both can result in ArrayIndexOutOfBoundException, in case the keySet()
    // is modified while this method is executed.
    // Therefore we must clone the set in the following manner, never relying on
    // keySet() size (even though it's used, ArrayList grows as needed.
    Set<String> fileNames = fileMap.keySet();
    List<String> names = new ArrayList<String>(fileNames.size());
    for (String name : fileNames) names.add(name);
    return names.toArray(new String[names.size()]);
{code}

> Use ConcurrentHashMap in RAMDirectory
> -------------------------------------
>
>                 Key: LUCENE-2779
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2779
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Store
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 3.1, 4.0
>
>         Attachments: LUCENE-2779-backwardsfix.patch, LUCENE-2779.patch, LUCENE-2779.patch,
LUCENE-2779.patch, TestCHM.java
>
>
> RAMDirectory synchronizes on its instance in many places to protect access to map of
RAMFiles, in addition to updating the sizeInBytes member. In many places the sync is done
for 'read' purposes, while only in few places we need 'write' access. This looks like a perfect
use case for ConcurrentHashMap
> Also, syncing around sizeInBytes is unnecessary IMO, since it's an AtomicLong ...
> I'll post a patch shortly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message