[
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: [email protected]
For additional commands, e-mail: [email protected]
