LUCENE-5681: Fix RAMDirectory's IndexInput to not do double buffering on slices (causes useless data copying, especially on random access slices).

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1610929 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -147,6 +147,13 @@ Optimizations
   of collecting all terms from the like text when building the query. 
   (Alex Ksikes, Simon Willnauer)
 
+* LUCENE-5681: Fix RAMDirectory's IndexInput to not do double buffering
+  on slices (causes useless data copying, especially on random access slices).
+  This also improves slices of NRTCachingDirectory, because the cache
+  is based on RAMDirectory. BufferedIndexInput.wrap() was marked with a
+  warning in javadocs. It is almost always a better idea to implement
+  slicing on your own!  (Uwe Schindler, Robert Muir)
+
 Bug Fixes
 
 * LUCENE-5796: Fixes the Scorer.getChildren() method for two combinations 

lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java

@@ -389,7 +389,7 @@ public abstract class BufferedIndexInput extends IndexInput implements RandomAcc
   
   @Override
   public IndexInput slice(String sliceDescription, long offset, long length) throws IOException {
-    return wrap("SlicedIndexInput(" + sliceDescription + " in " + this + ")", this, offset, length);
+    return wrap(sliceDescription, this, offset, length);
   }
 
   /**
@@ -426,10 +426,11 @@ public abstract class BufferedIndexInput extends IndexInput implements RandomAcc
   }
   
   /** 
-   * Wraps a portion of a file with buffering. 
+   * Wraps a portion of another IndexInput with buffering.
+   * <p><b>Please note:</b> This is in most cases ineffective, because it may double buffer!
    */
-  public static BufferedIndexInput wrap(String description, IndexInput other, long offset, long length) {
-    return new SlicedIndexInput(description, other, offset, length);
+  public static BufferedIndexInput wrap(String sliceDescription, IndexInput other, long offset, long length) {
+    return new SlicedIndexInput(sliceDescription, other, offset, length);
   }
   
   /** 
@@ -441,11 +442,10 @@ public abstract class BufferedIndexInput extends IndexInput implements RandomAcc
     long length;
     
     SlicedIndexInput(String sliceDescription, IndexInput base, long offset, long length) {
-      super("SlicedIndexInput(" + sliceDescription + " in " + base + " slice=" + offset + ":" + (offset+length) + ")", BufferedIndexInput.BUFFER_SIZE);
-      if (offset < 0 || length < 0) {
-        throw new IllegalArgumentException();
+      super((sliceDescription == null) ? base.toString() : (base.toString() + " [slice=" + sliceDescription + "]"), BufferedIndexInput.BUFFER_SIZE);
+      if (offset < 0 || length < 0 || offset + length > base.length()) {
+        throw new IllegalArgumentException("slice() " + sliceDescription + " out of bounds: "  + base);
       }
-      assert offset + length <= base.length();
       this.base = base.clone();
       this.fileOffset = offset;
       this.length = length;

lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java

@@ -26,8 +26,8 @@ import java.io.EOFException;
 public class RAMInputStream extends IndexInput implements Cloneable {
   static final int BUFFER_SIZE = RAMOutputStream.BUFFER_SIZE;
 
-  private RAMFile file;
-  private long length;
+  private final RAMFile file;
+  private final long length;
 
   private byte[] currentBuffer;
   private int currentBufferIndex;
@@ -37,9 +37,13 @@ public class RAMInputStream extends IndexInput implements Cloneable {
   private int bufferLength;
 
   public RAMInputStream(String name, RAMFile f) throws IOException {
+    this(name, f, f.length);
+  }
+
+  RAMInputStream(String name, RAMFile f, long length) throws IOException {
     super("RAMInputStream(name=" + name + ")");
-    file = f;
-    length = file.length;
+    this.file = f;
+    this.length = length;
     if (length/BUFFER_SIZE >= Integer.MAX_VALUE) {
       throw new IOException("RAMInputStream too large length=" + length + ": " + name); 
     }
@@ -88,7 +92,7 @@ public class RAMInputStream extends IndexInput implements Cloneable {
 
   private final void switchCurrentBuffer(boolean enforceEOF) throws IOException {
     bufferStart = (long) BUFFER_SIZE * (long) currentBufferIndex;
-    if (currentBufferIndex >= file.numBuffers()) {
+    if (bufferStart > length || currentBufferIndex >= file.numBuffers()) {
       // end of file reached, no more buffers left
       if (enforceEOF) {
         throw new EOFException("read past EOF: " + this);
@@ -119,9 +123,39 @@ public class RAMInputStream extends IndexInput implements Cloneable {
     bufferPosition = (int) (pos % BUFFER_SIZE);
   }
 
-  // TODO: improve this, kinda stupid
   @Override
-  public IndexInput slice(String sliceDescription, long offset, long length) throws IOException {
-    return BufferedIndexInput.wrap(sliceDescription, this, offset, length);
+  public IndexInput slice(String sliceDescription, final long offset, final long length) throws IOException {
+    if (offset < 0 || length < 0 || offset + length > this.length) {
+      throw new IllegalArgumentException("slice() " + sliceDescription + " out of bounds: "  + this);
+    }
+    final String newResourceDescription = (sliceDescription == null) ? toString() : (toString() + " [slice=" + sliceDescription + "]");
+    return new RAMInputStream(newResourceDescription, file, offset + length) {
+      {
+        seek(0L);
+      }
+      
+      @Override
+      public void seek(long pos) throws IOException {
+        if (pos < 0L) {
+          throw new IllegalArgumentException("Seeking to negative position: " + this);
+        }
+        super.seek(pos + offset);
+      }
+      
+      @Override
+      public long getFilePointer() {
+        return super.getFilePointer() - offset;
+      }
+
+      @Override
+      public long length() {
+        return super.length() - offset;
+      }
+
+      @Override
+      public IndexInput slice(String sliceDescription, long ofs, long len) throws IOException {
+        return super.slice(sliceDescription, offset + ofs, len);
+      }
+    };
   }
 }

lucene/core/src/java/org/apache/lucene/store/RAMOutputStream.java

@@ -31,7 +31,7 @@ import org.apache.lucene.util.Accountable;
 public class RAMOutputStream extends IndexOutput implements Accountable {
   static final int BUFFER_SIZE = 1024;
 
-  private RAMFile file;
+  private final RAMFile file;
 
   private byte[] currentBuffer;
   private int currentBufferIndex;

lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java

@@ -17,6 +17,7 @@ package org.apache.lucene.store;
  * limitations under the License.
  */
 
+import java.io.EOFException;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -439,6 +440,25 @@ public abstract class BaseDirectoryTestCase extends LuceneTestCase {
     dir.close();
   }
 
+  public void testSeekPastEOF() throws Exception {
+    Directory dir = getDirectory(createTempDir("testSeekPastEOF"));
+    IndexOutput o = dir.createOutput("out", newIOContext(random()));
+    final int len = random().nextInt(2048);
+    byte[] b = new byte[len];
+    o.writeBytes(b, 0, len);
+    o.close();
+    IndexInput i = dir.openInput("out", newIOContext(random()));
+    try {
+      i.seek(len + random().nextInt(2048));
+      i.readByte();
+      fail("Did not get EOFException");
+    } catch (EOFException eof) {
+      // pass
+    }
+    i.close();
+    dir.close();
+  }
+
   // LUCENE-3382 -- make sure we get exception if the directory really does not exist.
   public void testNoDir() throws Throwable {
     File tempDir = createTempDir("doesnotexist");