You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2010/11/29 16:18:43 UTC

svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Author: shaie
Date: Mon Nov 29 15:18:42 2010
New Revision: 1040145

URL: http://svn.apache.org/viewvc?rev=1040145&view=rev
Log:
LUCENE-2779: Use ConcurrentHashMap in RAMDirectory (3x)

Modified:
    lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Modified: lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java?rev=1040145&r1=1040144&r2=1040145&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java (original)
+++ lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java Mon Nov 29 15:18:42 2010
@@ -20,8 +20,8 @@ package org.apache.lucene.store;
 import java.io.IOException;
 import java.io.FileNotFoundException;
 import java.io.Serializable;
-import java.util.HashMap;
-import java.util.Set;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.lucene.index.IndexFileNameFilter;
@@ -36,7 +36,7 @@ public class RAMDirectory extends Direct
 
   private static final long serialVersionUID = 1l;
 
-  protected HashMap<String,RAMFile> fileMap = new HashMap<String,RAMFile>();
+  protected Map<String,RAMFile> fileMap = new ConcurrentHashMap<String,RAMFile>();
   protected final AtomicLong sizeInBytes = new AtomicLong();
   
   // *****
@@ -83,25 +83,16 @@ public class RAMDirectory extends Direct
   }
 
   @Override
-  public synchronized final String[] listAll() {
+  public final String[] listAll() {
     ensureOpen();
-    Set<String> fileNames = fileMap.keySet();
-    String[] result = new String[fileNames.size()];
-    int i = 0;
-    for(final String fileName: fileNames) 
-      result[i++] = fileName;
-    return result;
+    return fileMap.keySet().toArray(new String[0]);
   }
 
   /** Returns true iff the named file exists in this directory. */
   @Override
   public final boolean fileExists(String name) {
     ensureOpen();
-    RAMFile file;
-    synchronized (this) {
-      file = fileMap.get(name);
-    }
-    return file != null;
+    return fileMap.containsKey(name);
   }
 
   /** Returns the time the named file was last modified.
@@ -110,12 +101,10 @@ public class RAMDirectory extends Direct
   @Override
   public final long fileModified(String name) throws IOException {
     ensureOpen();
-    RAMFile file;
-    synchronized (this) {
-      file = fileMap.get(name);
-    }
-    if (file==null)
+    RAMFile file = fileMap.get(name);
+    if (file == null) {
       throw new FileNotFoundException(name);
+    }
     return file.getLastModified();
   }
 
@@ -125,12 +114,10 @@ public class RAMDirectory extends Direct
   @Override
   public void touchFile(String name) throws IOException {
     ensureOpen();
-    RAMFile file;
-    synchronized (this) {
-      file = fileMap.get(name);
-    }
-    if (file==null)
+    RAMFile file = fileMap.get(name);
+    if (file == null) {
       throw new FileNotFoundException(name);
+    }
     
     long ts2, ts1 = System.currentTimeMillis();
     do {
@@ -151,19 +138,18 @@ public class RAMDirectory extends Direct
   @Override
   public final long fileLength(String name) throws IOException {
     ensureOpen();
-    RAMFile file;
-    synchronized (this) {
-      file = fileMap.get(name);
-    }
-    if (file==null)
+    RAMFile file = fileMap.get(name);
+    if (file == null) {
       throw new FileNotFoundException(name);
+    }
     return file.getLength();
   }
   
-  /** Return total size in bytes of all files in this
-   * directory.  This is currently quantized to
-   * RAMOutputStream.BUFFER_SIZE. */
-  public synchronized final long sizeInBytes() {
+  /**
+   * Return total size in bytes of all files in this directory. This is
+   * currently quantized to RAMOutputStream.BUFFER_SIZE.
+   */
+  public final long sizeInBytes() {
     ensureOpen();
     return sizeInBytes.get();
   }
@@ -172,14 +158,15 @@ public class RAMDirectory extends Direct
    * @throws IOException if the file does not exist
    */
   @Override
-  public synchronized void deleteFile(String name) throws IOException {
+  public void deleteFile(String name) throws IOException {
     ensureOpen();
     RAMFile file = fileMap.remove(name);
-    if (file!=null) {
-        file.directory = null;
-        sizeInBytes.addAndGet(-file.sizeInBytes);
-    } else
+    if (file != null) {
+      file.directory = null;
+      sizeInBytes.addAndGet(-file.sizeInBytes);
+    } else {
       throw new FileNotFoundException(name);
+    }
   }
 
   /** Creates a new, empty file in the directory with the given name. Returns a stream writing this file. */
@@ -187,14 +174,12 @@ public class RAMDirectory extends Direct
   public IndexOutput createOutput(String name) throws IOException {
     ensureOpen();
     RAMFile file = newRAMFile();
-    synchronized (this) {
-      RAMFile existing = fileMap.get(name);
-      if (existing!=null) {
-        sizeInBytes.addAndGet(-existing.sizeInBytes);
-        existing.directory = null;
-      }
-      fileMap.put(name, file);
+    RAMFile existing = fileMap.remove(name);
+    if (existing != null) {
+      sizeInBytes.addAndGet(-existing.sizeInBytes);
+      existing.directory = null;
     }
+    fileMap.put(name, file);
     return new RAMOutputStream(file);
   }
 
@@ -211,12 +196,10 @@ public class RAMDirectory extends Direct
   @Override
   public IndexInput openInput(String name) throws IOException {
     ensureOpen();
-    RAMFile file;
-    synchronized (this) {
-      file = fileMap.get(name);
-    }
-    if (file == null)
+    RAMFile file = fileMap.get(name);
+    if (file == null) {
       throw new FileNotFoundException(name);
+    }
     return new RAMInputStream(file);
   }
 



Re: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Earwin Burrfoot <ea...@gmail.com>.
On Tue, Nov 30, 2010 at 07:48, Shai Erera <se...@gmail.com> wrote:
> The break was only in MockRAMDir, and even
> that is because I changed fileMap type from HashMap to Map, which IMO should
> have been defined like that from the beginning.
As a piece of trivia.
I did some benchmarks and declaring fields with the type of a concrete
subclass, rather than interface, proved to be faster.
(On a latest 1.6 Sun JVM)
I guess that's due to JIT's failure to inline method calls on that
field (or additional class checks introduced before the short path).
Ugly :/

-- 
Kirill Zakharenko/Кирилл Захаренко (earwin@gmail.com)
Phone: +7 (495) 683-567-4
ICQ: 104465785

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


Re: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Shai Erera <se...@gmail.com>.
Yes, good point - I'll make the member final. And also re-apply my patch
combined w/ yours.

Thanks
Shai

On Tue, Nov 30, 2010 at 9:31 AM, Uwe Schindler <uw...@thetaphi.de> wrote:

> --- From: Shai Erera [mailto:serera@gmail.com] ---
> Uwe, I'm sorry people rushed in to believe I broke backwards ... because I
> don't think I did. My only mistake was that I didn't run test-backwards,
> 'cause I didn't really think anything can break by using CHM and not HM.
>
> fileMap was package-private until two days ago (LUCENE-2778) when I made it
> protected - therefore it wasn't a public API, right? If it wasn't public,
> we're allowed to change it right? The break was only in MockRAMDir, and
> even
> that is because I changed fileMap type from HashMap to Map, which IMO
> should
> have been defined like that from the beginning.
> ---
>
> Hi Shai,
>
> Sorry I did not know that this field was package private before, I was only
> looking at the test. I am then fine with the patch. I have seen the other
> mail about backwards breaks and backwards tests - here the fix for this
> stupid HashMap/Map issue is easy, simply apply the BW patch attached to the
> issue.
>
> One thing: As you made the map protected in the previous issue, and it is
> never changed, for API safety it should be final in all cases! Can you
> change that, too. The reference to the Map should never be changed.
>
> See also my comments to the other mail!
>
> Uwe
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

RE: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Uwe Schindler <uw...@thetaphi.de>.
--- From: Shai Erera [mailto:serera@gmail.com] ---
Uwe, I'm sorry people rushed in to believe I broke backwards ... because I
don't think I did. My only mistake was that I didn't run test-backwards,
'cause I didn't really think anything can break by using CHM and not HM.

fileMap was package-private until two days ago (LUCENE-2778) when I made it
protected - therefore it wasn't a public API, right? If it wasn't public,
we're allowed to change it right? The break was only in MockRAMDir, and even
that is because I changed fileMap type from HashMap to Map, which IMO should
have been defined like that from the beginning.
---

Hi Shai,

Sorry I did not know that this field was package private before, I was only
looking at the test. I am then fine with the patch. I have seen the other
mail about backwards breaks and backwards tests - here the fix for this
stupid HashMap/Map issue is easy, simply apply the BW patch attached to the
issue.

One thing: As you made the map protected in the previous issue, and it is
never changed, for API safety it should be final in all cases! Can you
change that, too. The reference to the Map should never be changed.

See also my comments to the other mail!

Uwe



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


Re: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Shai Erera <se...@gmail.com>.
Uwe, I'm sorry people rushed in to believe I broke backwards ... because I
don't think I did. My only mistake was that I didn't run test-backwards,
'cause I didn't really think anything can break by using CHM and not HM.

fileMap was package-private until two days ago (LUCENE-2778) when I made it
protected - therefore it wasn't a public API, right? If it wasn't public,
we're allowed to change it right? The break was only in MockRAMDir, and even
that is because I changed fileMap type from HashMap to Map, which IMO should
have been defined like that from the beginning.

Perhaps we should discuss a more general problem we have w/ the backwards
test (maybe in a dedicated thread) -- classes that access package-private
API can break if that API changes - bw-policy-wise we're allowed to make
such changes, but we cannot fix the backwards layer to compile against it
properly. Your hack is really a hack, and shouldn't be there IMO. Instead,
fixing backwards is as simple as re-compiling it. Something we could have
done when backwards had its own 'src/java' folder which we could change for
situations like that.

So the question now is - does this still count as a backwards break? I don't
think so. I don't mind if we commit your hack to MockRAMDir just to avoid
Hudson from failing. But we need to think how we can make backwards more
resilient to changes that are allowed (such as changing package-private
API).

Shai

On Tue, Nov 30, 2010 at 12:07 AM, Uwe Schindler <uw...@thetaphi.de> wrote:

> Attached patch fixes the backwards tests...
>
> ********* BUT ***********
>
> The change in RAMDirectory of a *protected* field brakes backwards!!!! We
> are no longer binary compatible, so I don’t think the change is good in 3.x.
> We should do this only in trunk, so please revert the 3.x change for now or
> have a sophisticated backwards layer!
>
> ********* BUT ***********
>
> Uwe
>
> -----
> Uwe Schindler
> H.-H.-Meier-Allee 63, D-28213 Bremen
> http://www.thetaphi.de
> eMail: uwe@thetaphi.de
>
> > From: Uwe Schindler [mailto:uwe@thetaphi.de]
> > Sent: Monday, November 29, 2010 10:05 PM
> > To: dev@lucene.apache.org
> > Subject: RE: svn commit: r1040145 -
> > /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store
> > /RAMDirectory.java
> >
> > This commit broke backwards in MockRAMDir. A fix would be to change
> > MockRAMDir in backwards to use reflection to get the Map (like used at
> > another place in MockRAMDir already).
> >
> > Uwe
> >
> > -----
> > Uwe Schindler
> > H.-H.-Meier-Allee 63, D-28213 Bremen
> > http://www.thetaphi.de
> > eMail: uwe@thetaphi.de
> >
> >
> > > -----Original Message-----
> > > From: shaie@apache.org [mailto:shaie@apache.org]
> > > Sent: Monday, November 29, 2010 4:19 PM
> > > To: commits@lucene.apache.org
> > > Subject: svn commit: r1040145 -
> > >
> > /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store
> > > /RAMDirectory.java
> > >
> > > Author: shaie
> > > Date: Mon Nov 29 15:18:42 2010
> > > New Revision: 1040145
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1040145&view=rev
> > > Log:
> > > LUCENE-2779: Use ConcurrentHashMap in RAMDirectory (3x)
> > >
> > > Modified:
> > >
> > >
> > lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > > RAMDirectory.java
> > >
> > > Modified:
> > >
> > lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > > RAMDirectory.java
> > > URL:
> > >
> > http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/
> > >
> > java/org/apache/lucene/store/RAMDirectory.java?rev=1040145&r1=1040144
> > > &r2=1040145&view=diff
> > >
> > ==========================================================
> > > ====================
> > > ---
> > >
> > lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > > RAMDirectory.java (original)
> > > +++
> > >
> > lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > > RAMDirectory.java Mon Nov 29 15:18:42 2010 @@ -20,8 +20,8 @@ package
> > > org.apache.lucene.store;  import java.io.IOException;  import
> > > java.io.FileNotFoundException;  import java.io.Serializable; -import
> > > java.util.HashMap; -import java.util.Set;
> > > +import java.util.Map;
> > > +import java.util.concurrent.ConcurrentHashMap;
> > >  import java.util.concurrent.atomic.AtomicLong;
> > >
> > >  import org.apache.lucene.index.IndexFileNameFilter;
> > > @@ -36,7 +36,7 @@ public class RAMDirectory extends Direct
> > >
> > >    private static final long serialVersionUID = 1l;
> > >
> > > -  protected HashMap<String,RAMFile> fileMap = new
> > > HashMap<String,RAMFile>();
> > > +  protected Map<String,RAMFile> fileMap = new
> > > ConcurrentHashMap<String,RAMFile>();
> > >    protected final AtomicLong sizeInBytes = new AtomicLong();
> > >
> > >    // *****
> > > @@ -83,25 +83,16 @@ public class RAMDirectory extends Direct
> > >    }
> > >
> > >    @Override
> > > -  public synchronized final String[] listAll() {
> > > +  public final String[] listAll() {
> > >      ensureOpen();
> > > -    Set<String> fileNames = fileMap.keySet();
> > > -    String[] result = new String[fileNames.size()];
> > > -    int i = 0;
> > > -    for(final String fileName: fileNames)
> > > -      result[i++] = fileName;
> > > -    return result;
> > > +    return fileMap.keySet().toArray(new String[0]);
> > >    }
> > >
> > >    /** Returns true iff the named file exists in this directory. */
> > >    @Override
> > >    public final boolean fileExists(String name) {
> > >      ensureOpen();
> > > -    RAMFile file;
> > > -    synchronized (this) {
> > > -      file = fileMap.get(name);
> > > -    }
> > > -    return file != null;
> > > +    return fileMap.containsKey(name);
> > >    }
> > >
> > >    /** Returns the time the named file was last modified.
> > > @@ -110,12 +101,10 @@ public class RAMDirectory extends Direct
> > >    @Override
> > >    public final long fileModified(String name) throws IOException {
> > >      ensureOpen();
> > > -    RAMFile file;
> > > -    synchronized (this) {
> > > -      file = fileMap.get(name);
> > > -    }
> > > -    if (file==null)
> > > +    RAMFile file = fileMap.get(name);
> > > +    if (file == null) {
> > >        throw new FileNotFoundException(name);
> > > +    }
> > >      return file.getLastModified();
> > >    }
> > >
> > > @@ -125,12 +114,10 @@ public class RAMDirectory extends Direct
> > >    @Override
> > >    public void touchFile(String name) throws IOException {
> > >      ensureOpen();
> > > -    RAMFile file;
> > > -    synchronized (this) {
> > > -      file = fileMap.get(name);
> > > -    }
> > > -    if (file==null)
> > > +    RAMFile file = fileMap.get(name);
> > > +    if (file == null) {
> > >        throw new FileNotFoundException(name);
> > > +    }
> > >
> > >      long ts2, ts1 = System.currentTimeMillis();
> > >      do {
> > > @@ -151,19 +138,18 @@ public class RAMDirectory extends Direct
> > >    @Override
> > >    public final long fileLength(String name) throws IOException {
> > >      ensureOpen();
> > > -    RAMFile file;
> > > -    synchronized (this) {
> > > -      file = fileMap.get(name);
> > > -    }
> > > -    if (file==null)
> > > +    RAMFile file = fileMap.get(name);
> > > +    if (file == null) {
> > >        throw new FileNotFoundException(name);
> > > +    }
> > >      return file.getLength();
> > >    }
> > >
> > > -  /** Return total size in bytes of all files in this
> > > -   * directory.  This is currently quantized to
> > > -   * RAMOutputStream.BUFFER_SIZE. */
> > > -  public synchronized final long sizeInBytes() {
> > > +  /**
> > > +   * Return total size in bytes of all files in this directory. This
> is
> > > +   * currently quantized to RAMOutputStream.BUFFER_SIZE.
> > > +   */
> > > +  public final long sizeInBytes() {
> > >      ensureOpen();
> > >      return sizeInBytes.get();
> > >    }
> > > @@ -172,14 +158,15 @@ public class RAMDirectory extends Direct
> > >     * @throws IOException if the file does not exist
> > >     */
> > >    @Override
> > > -  public synchronized void deleteFile(String name) throws IOException
> > > {
> > > +  public void deleteFile(String name) throws IOException {
> > >      ensureOpen();
> > >      RAMFile file = fileMap.remove(name);
> > > -    if (file!=null) {
> > > -        file.directory = null;
> > > -        sizeInBytes.addAndGet(-file.sizeInBytes);
> > > -    } else
> > > +    if (file != null) {
> > > +      file.directory = null;
> > > +      sizeInBytes.addAndGet(-file.sizeInBytes);
> > > +    } else {
> > >        throw new FileNotFoundException(name);
> > > +    }
> > >    }
> > >
> > >    /** Creates a new, empty file in the directory with the given name.
> > > Returns a stream writing this file. */ @@ -187,14 +174,12 @@ public
> > > class RAMDirectory extends Direct
> > >    public IndexOutput createOutput(String name) throws IOException {
> > >      ensureOpen();
> > >      RAMFile file = newRAMFile();
> > > -    synchronized (this) {
> > > -      RAMFile existing = fileMap.get(name);
> > > -      if (existing!=null) {
> > > -        sizeInBytes.addAndGet(-existing.sizeInBytes);
> > > -        existing.directory = null;
> > > -      }
> > > -      fileMap.put(name, file);
> > > +    RAMFile existing = fileMap.remove(name);
> > > +    if (existing != null) {
> > > +      sizeInBytes.addAndGet(-existing.sizeInBytes);
> > > +      existing.directory = null;
> > >      }
> > > +    fileMap.put(name, file);
> > >      return new RAMOutputStream(file);
> > >    }
> > >
> > > @@ -211,12 +196,10 @@ public class RAMDirectory extends Direct
> > >    @Override
> > >    public IndexInput openInput(String name) throws IOException {
> > >      ensureOpen();
> > > -    RAMFile file;
> > > -    synchronized (this) {
> > > -      file = fileMap.get(name);
> > > -    }
> > > -    if (file == null)
> > > +    RAMFile file = fileMap.get(name);
> > > +    if (file == null) {
> > >        throw new FileNotFoundException(name);
> > > +    }
> > >      return new RAMInputStream(file);
> > >    }
> > >
> > >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org For additional
> > commands, e-mail: dev-help@lucene.apache.org
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>

RE: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Uwe Schindler <uw...@thetaphi.de>.
Attached patch fixes the backwards tests...

********* BUT ***********

The change in RAMDirectory of a *protected* field brakes backwards!!!! We are no longer binary compatible, so I don’t think the change is good in 3.x. We should do this only in trunk, so please revert the 3.x change for now or have a sophisticated backwards layer!

********* BUT ***********

Uwe

-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de

> From: Uwe Schindler [mailto:uwe@thetaphi.de]
> Sent: Monday, November 29, 2010 10:05 PM
> To: dev@lucene.apache.org
> Subject: RE: svn commit: r1040145 -
> /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store
> /RAMDirectory.java
> 
> This commit broke backwards in MockRAMDir. A fix would be to change
> MockRAMDir in backwards to use reflection to get the Map (like used at
> another place in MockRAMDir already).
> 
> Uwe
> 
> -----
> Uwe Schindler
> H.-H.-Meier-Allee 63, D-28213 Bremen
> http://www.thetaphi.de
> eMail: uwe@thetaphi.de
> 
> 
> > -----Original Message-----
> > From: shaie@apache.org [mailto:shaie@apache.org]
> > Sent: Monday, November 29, 2010 4:19 PM
> > To: commits@lucene.apache.org
> > Subject: svn commit: r1040145 -
> >
> /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store
> > /RAMDirectory.java
> >
> > Author: shaie
> > Date: Mon Nov 29 15:18:42 2010
> > New Revision: 1040145
> >
> > URL: http://svn.apache.org/viewvc?rev=1040145&view=rev
> > Log:
> > LUCENE-2779: Use ConcurrentHashMap in RAMDirectory (3x)
> >
> > Modified:
> >
> >
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > RAMDirectory.java
> >
> > Modified:
> >
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > RAMDirectory.java
> > URL:
> >
> http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/
> >
> java/org/apache/lucene/store/RAMDirectory.java?rev=1040145&r1=1040144
> > &r2=1040145&view=diff
> >
> ==========================================================
> > ====================
> > ---
> >
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > RAMDirectory.java (original)
> > +++
> >
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> > RAMDirectory.java Mon Nov 29 15:18:42 2010 @@ -20,8 +20,8 @@ package
> > org.apache.lucene.store;  import java.io.IOException;  import
> > java.io.FileNotFoundException;  import java.io.Serializable; -import
> > java.util.HashMap; -import java.util.Set;
> > +import java.util.Map;
> > +import java.util.concurrent.ConcurrentHashMap;
> >  import java.util.concurrent.atomic.AtomicLong;
> >
> >  import org.apache.lucene.index.IndexFileNameFilter;
> > @@ -36,7 +36,7 @@ public class RAMDirectory extends Direct
> >
> >    private static final long serialVersionUID = 1l;
> >
> > -  protected HashMap<String,RAMFile> fileMap = new
> > HashMap<String,RAMFile>();
> > +  protected Map<String,RAMFile> fileMap = new
> > ConcurrentHashMap<String,RAMFile>();
> >    protected final AtomicLong sizeInBytes = new AtomicLong();
> >
> >    // *****
> > @@ -83,25 +83,16 @@ public class RAMDirectory extends Direct
> >    }
> >
> >    @Override
> > -  public synchronized final String[] listAll() {
> > +  public final String[] listAll() {
> >      ensureOpen();
> > -    Set<String> fileNames = fileMap.keySet();
> > -    String[] result = new String[fileNames.size()];
> > -    int i = 0;
> > -    for(final String fileName: fileNames)
> > -      result[i++] = fileName;
> > -    return result;
> > +    return fileMap.keySet().toArray(new String[0]);
> >    }
> >
> >    /** Returns true iff the named file exists in this directory. */
> >    @Override
> >    public final boolean fileExists(String name) {
> >      ensureOpen();
> > -    RAMFile file;
> > -    synchronized (this) {
> > -      file = fileMap.get(name);
> > -    }
> > -    return file != null;
> > +    return fileMap.containsKey(name);
> >    }
> >
> >    /** Returns the time the named file was last modified.
> > @@ -110,12 +101,10 @@ public class RAMDirectory extends Direct
> >    @Override
> >    public final long fileModified(String name) throws IOException {
> >      ensureOpen();
> > -    RAMFile file;
> > -    synchronized (this) {
> > -      file = fileMap.get(name);
> > -    }
> > -    if (file==null)
> > +    RAMFile file = fileMap.get(name);
> > +    if (file == null) {
> >        throw new FileNotFoundException(name);
> > +    }
> >      return file.getLastModified();
> >    }
> >
> > @@ -125,12 +114,10 @@ public class RAMDirectory extends Direct
> >    @Override
> >    public void touchFile(String name) throws IOException {
> >      ensureOpen();
> > -    RAMFile file;
> > -    synchronized (this) {
> > -      file = fileMap.get(name);
> > -    }
> > -    if (file==null)
> > +    RAMFile file = fileMap.get(name);
> > +    if (file == null) {
> >        throw new FileNotFoundException(name);
> > +    }
> >
> >      long ts2, ts1 = System.currentTimeMillis();
> >      do {
> > @@ -151,19 +138,18 @@ public class RAMDirectory extends Direct
> >    @Override
> >    public final long fileLength(String name) throws IOException {
> >      ensureOpen();
> > -    RAMFile file;
> > -    synchronized (this) {
> > -      file = fileMap.get(name);
> > -    }
> > -    if (file==null)
> > +    RAMFile file = fileMap.get(name);
> > +    if (file == null) {
> >        throw new FileNotFoundException(name);
> > +    }
> >      return file.getLength();
> >    }
> >
> > -  /** Return total size in bytes of all files in this
> > -   * directory.  This is currently quantized to
> > -   * RAMOutputStream.BUFFER_SIZE. */
> > -  public synchronized final long sizeInBytes() {
> > +  /**
> > +   * Return total size in bytes of all files in this directory. This is
> > +   * currently quantized to RAMOutputStream.BUFFER_SIZE.
> > +   */
> > +  public final long sizeInBytes() {
> >      ensureOpen();
> >      return sizeInBytes.get();
> >    }
> > @@ -172,14 +158,15 @@ public class RAMDirectory extends Direct
> >     * @throws IOException if the file does not exist
> >     */
> >    @Override
> > -  public synchronized void deleteFile(String name) throws IOException
> > {
> > +  public void deleteFile(String name) throws IOException {
> >      ensureOpen();
> >      RAMFile file = fileMap.remove(name);
> > -    if (file!=null) {
> > -        file.directory = null;
> > -        sizeInBytes.addAndGet(-file.sizeInBytes);
> > -    } else
> > +    if (file != null) {
> > +      file.directory = null;
> > +      sizeInBytes.addAndGet(-file.sizeInBytes);
> > +    } else {
> >        throw new FileNotFoundException(name);
> > +    }
> >    }
> >
> >    /** Creates a new, empty file in the directory with the given name.
> > Returns a stream writing this file. */ @@ -187,14 +174,12 @@ public
> > class RAMDirectory extends Direct
> >    public IndexOutput createOutput(String name) throws IOException {
> >      ensureOpen();
> >      RAMFile file = newRAMFile();
> > -    synchronized (this) {
> > -      RAMFile existing = fileMap.get(name);
> > -      if (existing!=null) {
> > -        sizeInBytes.addAndGet(-existing.sizeInBytes);
> > -        existing.directory = null;
> > -      }
> > -      fileMap.put(name, file);
> > +    RAMFile existing = fileMap.remove(name);
> > +    if (existing != null) {
> > +      sizeInBytes.addAndGet(-existing.sizeInBytes);
> > +      existing.directory = null;
> >      }
> > +    fileMap.put(name, file);
> >      return new RAMOutputStream(file);
> >    }
> >
> > @@ -211,12 +196,10 @@ public class RAMDirectory extends Direct
> >    @Override
> >    public IndexInput openInput(String name) throws IOException {
> >      ensureOpen();
> > -    RAMFile file;
> > -    synchronized (this) {
> > -      file = fileMap.get(name);
> > -    }
> > -    if (file == null)
> > +    RAMFile file = fileMap.get(name);
> > +    if (file == null) {
> >        throw new FileNotFoundException(name);
> > +    }
> >      return new RAMInputStream(file);
> >    }
> >
> >
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org For additional
> commands, e-mail: dev-help@lucene.apache.org


RE: svn commit: r1040145 - /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/RAMDirectory.java

Posted by Uwe Schindler <uw...@thetaphi.de>.
This commit broke backwards in MockRAMDir. A fix would be to change MockRAMDir in backwards to use reflection to get the Map (like used at another place in MockRAMDir already).

Uwe

-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de


> -----Original Message-----
> From: shaie@apache.org [mailto:shaie@apache.org]
> Sent: Monday, November 29, 2010 4:19 PM
> To: commits@lucene.apache.org
> Subject: svn commit: r1040145 -
> /lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store
> /RAMDirectory.java
> 
> Author: shaie
> Date: Mon Nov 29 15:18:42 2010
> New Revision: 1040145
> 
> URL: http://svn.apache.org/viewvc?rev=1040145&view=rev
> Log:
> LUCENE-2779: Use ConcurrentHashMap in RAMDirectory (3x)
> 
> Modified:
> 
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> RAMDirectory.java
> 
> Modified:
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> RAMDirectory.java
> URL:
> http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/src/
> java/org/apache/lucene/store/RAMDirectory.java?rev=1040145&r1=1040144
> &r2=1040145&view=diff
> ==========================================================
> ====================
> ---
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> RAMDirectory.java (original)
> +++
> lucene/dev/branches/branch_3x/lucene/src/java/org/apache/lucene/store/
> RAMDirectory.java Mon Nov 29 15:18:42 2010
> @@ -20,8 +20,8 @@ package org.apache.lucene.store;
>  import java.io.IOException;
>  import java.io.FileNotFoundException;
>  import java.io.Serializable;
> -import java.util.HashMap;
> -import java.util.Set;
> +import java.util.Map;
> +import java.util.concurrent.ConcurrentHashMap;
>  import java.util.concurrent.atomic.AtomicLong;
> 
>  import org.apache.lucene.index.IndexFileNameFilter;
> @@ -36,7 +36,7 @@ public class RAMDirectory extends Direct
> 
>    private static final long serialVersionUID = 1l;
> 
> -  protected HashMap<String,RAMFile> fileMap = new
> HashMap<String,RAMFile>();
> +  protected Map<String,RAMFile> fileMap = new
> ConcurrentHashMap<String,RAMFile>();
>    protected final AtomicLong sizeInBytes = new AtomicLong();
> 
>    // *****
> @@ -83,25 +83,16 @@ public class RAMDirectory extends Direct
>    }
> 
>    @Override
> -  public synchronized final String[] listAll() {
> +  public final String[] listAll() {
>      ensureOpen();
> -    Set<String> fileNames = fileMap.keySet();
> -    String[] result = new String[fileNames.size()];
> -    int i = 0;
> -    for(final String fileName: fileNames)
> -      result[i++] = fileName;
> -    return result;
> +    return fileMap.keySet().toArray(new String[0]);
>    }
> 
>    /** Returns true iff the named file exists in this directory. */
>    @Override
>    public final boolean fileExists(String name) {
>      ensureOpen();
> -    RAMFile file;
> -    synchronized (this) {
> -      file = fileMap.get(name);
> -    }
> -    return file != null;
> +    return fileMap.containsKey(name);
>    }
> 
>    /** Returns the time the named file was last modified.
> @@ -110,12 +101,10 @@ public class RAMDirectory extends Direct
>    @Override
>    public final long fileModified(String name) throws IOException {
>      ensureOpen();
> -    RAMFile file;
> -    synchronized (this) {
> -      file = fileMap.get(name);
> -    }
> -    if (file==null)
> +    RAMFile file = fileMap.get(name);
> +    if (file == null) {
>        throw new FileNotFoundException(name);
> +    }
>      return file.getLastModified();
>    }
> 
> @@ -125,12 +114,10 @@ public class RAMDirectory extends Direct
>    @Override
>    public void touchFile(String name) throws IOException {
>      ensureOpen();
> -    RAMFile file;
> -    synchronized (this) {
> -      file = fileMap.get(name);
> -    }
> -    if (file==null)
> +    RAMFile file = fileMap.get(name);
> +    if (file == null) {
>        throw new FileNotFoundException(name);
> +    }
> 
>      long ts2, ts1 = System.currentTimeMillis();
>      do {
> @@ -151,19 +138,18 @@ public class RAMDirectory extends Direct
>    @Override
>    public final long fileLength(String name) throws IOException {
>      ensureOpen();
> -    RAMFile file;
> -    synchronized (this) {
> -      file = fileMap.get(name);
> -    }
> -    if (file==null)
> +    RAMFile file = fileMap.get(name);
> +    if (file == null) {
>        throw new FileNotFoundException(name);
> +    }
>      return file.getLength();
>    }
> 
> -  /** Return total size in bytes of all files in this
> -   * directory.  This is currently quantized to
> -   * RAMOutputStream.BUFFER_SIZE. */
> -  public synchronized final long sizeInBytes() {
> +  /**
> +   * Return total size in bytes of all files in this directory. This is
> +   * currently quantized to RAMOutputStream.BUFFER_SIZE.
> +   */
> +  public final long sizeInBytes() {
>      ensureOpen();
>      return sizeInBytes.get();
>    }
> @@ -172,14 +158,15 @@ public class RAMDirectory extends Direct
>     * @throws IOException if the file does not exist
>     */
>    @Override
> -  public synchronized void deleteFile(String name) throws IOException {
> +  public void deleteFile(String name) throws IOException {
>      ensureOpen();
>      RAMFile file = fileMap.remove(name);
> -    if (file!=null) {
> -        file.directory = null;
> -        sizeInBytes.addAndGet(-file.sizeInBytes);
> -    } else
> +    if (file != null) {
> +      file.directory = null;
> +      sizeInBytes.addAndGet(-file.sizeInBytes);
> +    } else {
>        throw new FileNotFoundException(name);
> +    }
>    }
> 
>    /** Creates a new, empty file in the directory with the given name. Returns
> a stream writing this file. */
> @@ -187,14 +174,12 @@ public class RAMDirectory extends Direct
>    public IndexOutput createOutput(String name) throws IOException {
>      ensureOpen();
>      RAMFile file = newRAMFile();
> -    synchronized (this) {
> -      RAMFile existing = fileMap.get(name);
> -      if (existing!=null) {
> -        sizeInBytes.addAndGet(-existing.sizeInBytes);
> -        existing.directory = null;
> -      }
> -      fileMap.put(name, file);
> +    RAMFile existing = fileMap.remove(name);
> +    if (existing != null) {
> +      sizeInBytes.addAndGet(-existing.sizeInBytes);
> +      existing.directory = null;
>      }
> +    fileMap.put(name, file);
>      return new RAMOutputStream(file);
>    }
> 
> @@ -211,12 +196,10 @@ public class RAMDirectory extends Direct
>    @Override
>    public IndexInput openInput(String name) throws IOException {
>      ensureOpen();
> -    RAMFile file;
> -    synchronized (this) {
> -      file = fileMap.get(name);
> -    }
> -    if (file == null)
> +    RAMFile file = fileMap.get(name);
> +    if (file == null) {
>        throw new FileNotFoundException(name);
> +    }
>      return new RAMInputStream(file);
>    }
> 
> 



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