[jira] Issue Comment Edited: (LUCENE-2779) Use ConcurrentHashMap in RAMDirectory

["Uwe Schindler (JIRA)" <ji...@apache.org>]

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

Uwe Schindler edited comment on LUCENE-2779 at 11/30/10 11:53 AM:
------------------------------------------------------------------

Yonik: You are right. Sun Java 5 creates an ArrayList and then uses the Iterator and adds each key to the ArrayList. After that it calls toArray on the ArrayList. This is safe, as the iterator is documented to be safe. Java 6 is safe as AbstractCollection.toArray() is oficially documented to behave correctly.

If Shai wants to keep this code, he should at least use ArrayList instead of HashSet for the keys, as cloning is much faster then. But as ArrayList(Collection) uses toArray() in its ctor it may also be broken.

Maybe Shai uses IBM JRocket? Or Harmony (Harmony is currently broken, HARMONY-6681)?

      was (Author: thetaphi):
    Yonik: You are right. In Sun Java 5 (src.zip, ConcurrentHashMap.java, line 1207) you can see how the toArray is implemented for KeySet. It creates an ArrayList and then uses the Iterator and adds each key to the ArrayList. After that it calls toArray on the ArrayList. This is safe, as the iterator is documented to be safe.

If Shai wants to keep this code, he should at least use ArrayList instead of HashSet for the keys, as cloning is much faster then.

Maybe Shai uses IBM JRocket? Or Harmony?
  
> 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