Index: lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java (revision 1512797) +++ lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java (working copy) @@ -23,7 +23,7 @@ /** Base implementation class for buffered {@link IndexInput}. */ public abstract class BufferedIndexInput extends IndexInput { - /** Default buffer size set to 1024*/ + /** Default buffer size set to {@value #BUFFER_SIZE}. */ public static final int BUFFER_SIZE = 1024; // The normal read buffer size defaults to 1024, but @@ -33,7 +33,7 @@ // BufferedIndexInputs created during merging. See // LUCENE-888 for details. /** - * A buffer size for merges set to 4096 + * A buffer size for merges set to {@value #MERGE_BUFFER_SIZE}. */ public static final int MERGE_BUFFER_SIZE = 4096; @@ -115,15 +115,14 @@ @Override public final void readBytes(byte[] b, int offset, int len, boolean useBuffer) throws IOException { - - if(len <= (bufferLength-bufferPosition)){ + int available = bufferLength - bufferPosition; + if(len <= available){ // the buffer contains enough data to satisfy this request if(len>0) // to allow b to be null if len is 0... System.arraycopy(buffer, bufferPosition, b, offset, len); bufferPosition+=len; } else { // the buffer does not have enough data. First serve all we've got. - int available = bufferLength - bufferPosition; if(available > 0){ System.arraycopy(buffer, bufferPosition, b, offset, available); offset += available; Index: lucene/core/src/java/org/apache/lucene/store/BufferedIndexOutput.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/BufferedIndexOutput.java (revision 1512797) +++ lucene/core/src/java/org/apache/lucene/store/BufferedIndexOutput.java (working copy) @@ -22,7 +22,7 @@ /** Base implementation class for buffered {@link IndexOutput}. */ public abstract class BufferedIndexOutput extends IndexOutput { /** The default buffer size in bytes ({@value #DEFAULT_BUFFER_SIZE}). */ - public static final int DEFAULT_BUFFER_SIZE = 16384; + public static final int DEFAULT_BUFFER_SIZE = 8192; private final int bufferSize; private final byte[] buffer; Index: lucene/core/src/java/org/apache/lucene/store/FSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/FSDirectory.java (revision 1512797) +++ lucene/core/src/java/org/apache/lucene/store/FSDirectory.java (working copy) @@ -31,6 +31,7 @@ import org.apache.lucene.util.ThreadInterruptedException; import org.apache.lucene.util.Constants; +import org.apache.lucene.util.IOUtils; /** * Base class for Directory implementations that store index @@ -112,13 +113,16 @@ public abstract class FSDirectory extends Directory { /** - * Default read chunk size: 2*{@link BufferedIndexInput#MERGE_BUFFER_SIZE}. + * Default read chunk size: 8192 bytes (this is the size up to which the JDK + does not allocate additional arrays while reading/writing) + @deprecated This constant is no longer used since Lucene 4.5. */ - public static final int DEFAULT_READ_CHUNK_SIZE = BufferedIndexInput.MERGE_BUFFER_SIZE * 2; + @Deprecated + public static final int DEFAULT_READ_CHUNK_SIZE = 8192; protected final File directory; // The underlying filesystem directory protected final Set staleFiles = synchronizedSet(new HashSet()); // Files written, but not yet sync'ed - private int chunkSize = DEFAULT_READ_CHUNK_SIZE; // LUCENE-1566 + private int chunkSize = DEFAULT_READ_CHUNK_SIZE; // returns the canonical version of the directory, creating it if it doesn't exist. private static File getCanonicalPath(File file) throws IOException { @@ -352,24 +356,11 @@ } /** - * Sets the maximum number of bytes read at once from the - * underlying file during {@link IndexInput#readBytes}. - * The default value is {@link #DEFAULT_READ_CHUNK_SIZE}; - * - *

This was introduced due to Sun - * JVM Bug 6478546, which throws an incorrect - * OutOfMemoryError when attempting to read too many bytes - * at once. It only happens on 32bit JVMs with a large - * maximum heap size.

- * - *

Changes to this value will not impact any - * already-opened {@link IndexInput}s. You should call - * this before attempting to open an index on the - * directory.

+ * This setting has no effect anymore. + * @deprecated This is no longer used since Lucene 4.5. */ + @Deprecated public final void setReadChunkSize(int chunkSize) { - // LUCENE-1566 if (chunkSize <= 0) { throw new IllegalArgumentException("chunkSize must be positive"); } @@ -377,12 +368,11 @@ } /** - * The maximum number of bytes to read at once from the - * underlying file during {@link IndexInput#readBytes}. - * @see #setReadChunkSize + * This setting has no effect anymore. + * @deprecated This is no longer used since Lucene 4.5. */ + @Deprecated public final int getReadChunkSize() { - // LUCENE-1566 return chunkSize; } @@ -390,6 +380,12 @@ * Writes output with {@link RandomAccessFile#write(byte[], int, int)} */ protected static class FSIndexOutput extends BufferedIndexOutput { + /** + * The maximum chunk size is 8192 bytes, because {@link RandomAccessFile} mallocs + * a native buffer outside of stack if the write buffer size is larger. + */ + private static final int CHUNK_SIZE = 8192; + private final FSDirectory parent; private final String name; private final RandomAccessFile file; @@ -402,11 +398,16 @@ isOpen = true; } - /** output methods: */ @Override - public void flushBuffer(byte[] b, int offset, int size) throws IOException { + protected void flushBuffer(byte[] b, int offset, int size) throws IOException { assert isOpen; - file.write(b, offset, size); + while (size > 0) { + final int toWrite = Math.min(CHUNK_SIZE, size); + file.write(b, offset, toWrite); + offset += toWrite; + size -= toWrite; + } + assert size == 0; } @Override @@ -414,21 +415,14 @@ parent.onIndexOutputClosed(name); // only close the file if it has not been closed yet if (isOpen) { - boolean success = false; + IOException priorE = null; try { super.close(); - success = true; + } catch (IOException ioe) { + priorE = ioe; } finally { isOpen = false; - if (!success) { - try { - file.close(); - } catch (Throwable t) { - // Suppress so we don't mask original exception - } - } else { - file.close(); - } + IOUtils.closeWhileHandlingException(priorE, file); } } } Index: lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java (revision 1512797) +++ lucene/core/src/java/org/apache/lucene/store/NIOFSDirectory.java (working copy) @@ -20,6 +20,7 @@ import java.io.File; import java.io.EOFException; import java.io.IOException; +import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; // javadoc @link import java.nio.channels.FileChannel; @@ -79,7 +80,7 @@ ensureOpen(); File path = new File(getDirectory(), name); FileChannel fc = FileChannel.open(path.toPath(), StandardOpenOption.READ); - return new NIOFSIndexInput("NIOFSIndexInput(path=\"" + path + "\")", fc, context, getReadChunkSize()); + return new NIOFSIndexInput("NIOFSIndexInput(path=\"" + path + "\")", fc, context); } @Override @@ -98,7 +99,7 @@ @Override public IndexInput openSlice(String sliceDescription, long offset, long length) { return new NIOFSIndexInput("NIOFSIndexInput(" + sliceDescription + " in path=\"" + path + "\" slice=" + offset + ":" + (offset+length) + ")", descriptor, offset, - length, BufferedIndexInput.bufferSize(context), getReadChunkSize()); + length, BufferedIndexInput.bufferSize(context)); } }; } @@ -107,12 +108,15 @@ * Reads bytes with {@link FileChannel#read(ByteBuffer, long)} */ protected static class NIOFSIndexInput extends BufferedIndexInput { + /** + * The maximum chunk size for reads of 16384 bytes. + */ + private static final int CHUNK_SIZE = 16384; + /** the file channel we will read from */ protected final FileChannel channel; /** is this instance a clone and hence does not own the file to close it */ boolean isClone = false; - /** maximum read length on a 32bit JVM to prevent incorrect OOM, see LUCENE-1566 */ - protected final int chunkSize; /** start offset: non-zero in the slice case */ protected final long off; /** end offset (start+length) */ @@ -120,18 +124,16 @@ private ByteBuffer byteBuf; // wraps the buffer for NIO - public NIOFSIndexInput(String resourceDesc, FileChannel fc, IOContext context, int chunkSize) throws IOException { + public NIOFSIndexInput(String resourceDesc, FileChannel fc, IOContext context) throws IOException { super(resourceDesc, context); this.channel = fc; - this.chunkSize = chunkSize; this.off = 0L; this.end = fc.size(); } - public NIOFSIndexInput(String resourceDesc, FileChannel fc, long off, long length, int bufferSize, int chunkSize) { + public NIOFSIndexInput(String resourceDesc, FileChannel fc, long off, long length, int bufferSize) { super(resourceDesc, bufferSize); this.channel = fc; - this.chunkSize = chunkSize; this.off = off; this.end = off + length; this.isClone = true; @@ -164,7 +166,6 @@ @Override protected void readInternal(byte[] b, int offset, int len) throws IOException { - final ByteBuffer bb; // Determine the ByteBuffer we should use @@ -172,16 +173,11 @@ // Use our own pre-wrapped byteBuf: assert byteBuf != null; byteBuf.clear(); - byteBuf.limit(len); bb = byteBuf; } else { bb = ByteBuffer.wrap(b, offset, len); } - int readOffset = bb.position(); - int readLength = bb.limit() - readOffset; - assert readLength == len; - long pos = getFilePointer() + off; if (pos + len > end) { @@ -189,33 +185,19 @@ } try { + int readLength = len; while (readLength > 0) { - final int limit; - if (readLength > chunkSize) { - // LUCENE-1566 - work around JVM Bug by breaking - // very large reads into chunks - limit = readOffset + chunkSize; - } else { - limit = readOffset + readLength; + final int toRead = Math.min(CHUNK_SIZE, readLength); + bb.limit(bb.position() + toRead); + final int i = channel.read(bb, pos); + if (i < 0) { // be defensive here, even though we checked before hand, something could have changed + throw new EOFException("read past EOF: " + this + " off: " + offset + " len: " + len + " pos: " + pos + " chunkLen: " + toRead + " end: " + end); } - bb.limit(limit); - int i = channel.read(bb, pos); - if (i < 0){//be defensive here, even though we checked before hand, something could have changed - throw new EOFException("read past EOF: " + this + " off: " + offset + " len: " + len + " pos: " + pos + " limit: " + limit + " end: " + end); - } + assert i > 0 : "FileChannel.read with non zero-length bb.remaining() must always read at least one byte (FileChannel is in blocking mode, see spec of ReadableByteChannel)"; pos += i; - readOffset += i; readLength -= i; } - } catch (OutOfMemoryError e) { - // propagate OOM up and add a hint for 32bit VM Users hitting the bug - // with a large chunk size in the fast path. - final OutOfMemoryError outOfMemoryError = new OutOfMemoryError( - "OutOfMemoryError likely caused by the Sun VM Bug described in " - + "https://issues.apache.org/jira/browse/LUCENE-1566; try calling FSDirectory.setReadChunkSize " - + "with a value smaller than the current chunk size (" + chunkSize + ")"); - outOfMemoryError.initCause(e); - throw outOfMemoryError; + assert readLength == 0; } catch (IOException ioe) { throw new IOException(ioe.getMessage() + ": " + this, ioe); } Index: lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java =================================================================== --- lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java (revision 1512797) +++ lucene/core/src/java/org/apache/lucene/store/SimpleFSDirectory.java (working copy) @@ -56,7 +56,7 @@ ensureOpen(); final File path = new File(directory, name); RandomAccessFile raf = new RandomAccessFile(path, "r"); - return new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + path.getPath() + "\")", raf, context, getReadChunkSize()); + return new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + path.getPath() + "\")", raf, context); } @Override @@ -75,7 +75,7 @@ @Override public IndexInput openSlice(String sliceDescription, long offset, long length) { return new SimpleFSIndexInput("SimpleFSIndexInput(" + sliceDescription + " in path=\"" + file.getPath() + "\" slice=" + offset + ":" + (offset+length) + ")", descriptor, offset, - length, BufferedIndexInput.bufferSize(context), getReadChunkSize()); + length, BufferedIndexInput.bufferSize(context)); } }; } @@ -85,29 +85,31 @@ * {@link RandomAccessFile#read(byte[], int, int)}. */ protected static class SimpleFSIndexInput extends BufferedIndexInput { + /** + * The maximum chunk size is 8192 bytes, because {@link RandomAccessFile} mallocs + * a native buffer outside of stack if the read buffer size is larger. + */ + private static final int CHUNK_SIZE = 8192; + /** the file channel we will read from */ protected final RandomAccessFile file; /** is this instance a clone and hence does not own the file to close it */ boolean isClone = false; - /** maximum read length on a 32bit JVM to prevent incorrect OOM, see LUCENE-1566 */ - protected final int chunkSize; /** start offset: non-zero in the slice case */ protected final long off; /** end offset (start+length) */ protected final long end; - public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, IOContext context, int chunkSize) throws IOException { + public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, IOContext context) throws IOException { super(resourceDesc, context); this.file = file; - this.chunkSize = chunkSize; this.off = 0L; this.end = file.length(); } - public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, long off, long length, int bufferSize, int chunkSize) { + public SimpleFSIndexInput(String resourceDesc, RandomAccessFile file, long off, long length, int bufferSize) { super(resourceDesc, bufferSize); this.file = file; - this.chunkSize = chunkSize; this.off = off; this.end = off + length; this.isClone = true; @@ -146,29 +148,16 @@ } try { - do { - final int readLength; - if (total + chunkSize > len) { - readLength = len - total; - } else { - // LUCENE-1566 - work around JVM Bug by breaking very large reads into chunks - readLength = chunkSize; + while (total < len) { + final int toRead = Math.min(CHUNK_SIZE, len - total); + final int i = file.read(b, offset + total, toRead); + if (i < 0) { // be defensive here, even though we checked before hand, something could have changed + throw new EOFException("read past EOF: " + this + " off: " + offset + " len: " + len + " total: " + total + " chunkLen: " + toRead + " end: " + end); } - final int i = file.read(b, offset + total, readLength); - if (i < 0){//be defensive here, even though we checked before hand, something could have changed - throw new EOFException("read past EOF: " + this + " off: " + offset + " len: " + len + " total: " + total + " readLen: " + readLength + " end: " + end); - } + assert i > 0 : "RandomAccessFile.read with non zero-length toRead must always read at least one byte"; total += i; - } while (total < len); - } catch (OutOfMemoryError e) { - // propagate OOM up and add a hint for 32bit VM Users hitting the bug - // with a large chunk size in the fast path. - final OutOfMemoryError outOfMemoryError = new OutOfMemoryError( - "OutOfMemoryError likely caused by the Sun VM Bug described in " - + "https://issues.apache.org/jira/browse/LUCENE-1566; try calling FSDirectory.setReadChunkSize " - + "with a value smaller than the current chunk size (" + chunkSize + ")"); - outOfMemoryError.initCause(e); - throw outOfMemoryError; + } + assert total == len; } catch (IOException ioe) { throw new IOException(ioe.getMessage() + ": " + this, ioe); } Index: lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java =================================================================== --- lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java (revision 1512797) +++ lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java (working copy) @@ -84,25 +84,6 @@ public void testReadBytes() throws Exception { MyBufferedIndexInput input = new MyBufferedIndexInput(); runReadBytes(input, BufferedIndexInput.BUFFER_SIZE, random()); - - // This tests the workaround code for LUCENE-1566 where readBytesInternal - // provides a workaround for a JVM Bug that incorrectly raises a OOM Error - // when a large byte buffer is passed to a file read. - // NOTE: this does only test the chunked reads and NOT if the Bug is triggered. - //final int tmpFileSize = 1024 * 1024 * 5; - final int inputBufferSize = 128; - File tmpInputFile = _TestUtil.createTempFile("IndexInput", "tmpFile", TEMP_DIR); - tmpInputFile.deleteOnExit(); - writeBytes(tmpInputFile, TEST_FILE_LENGTH); - - // run test with chunk size of 10 bytes - runReadBytesAndClose(new SimpleFSIndexInput("SimpleFSIndexInput(path=\"" + tmpInputFile + "\")", - new RandomAccessFile(tmpInputFile, "r"), newIOContext(random()), 10), inputBufferSize, random()); - - // run test with chunk size of 10 bytes - runReadBytesAndClose(new NIOFSIndexInput("NIOFSIndexInput(path=\"" + tmpInputFile + "\")", - FileChannel.open(tmpInputFile.toPath(), StandardOpenOption.READ), newIOContext(random()), 10), - inputBufferSize, random()); } private void runReadBytesAndClose(IndexInput input, int bufferSize, Random r) throws IOException { @@ -211,6 +192,7 @@ private static byte byten(long n){ return (byte)(n*n%256); } + private static class MyBufferedIndexInput extends BufferedIndexInput { private long pos; private long len; Index: lucene/core/src/test/org/apache/lucene/store/TestDirectory.java =================================================================== --- lucene/core/src/test/org/apache/lucene/store/TestDirectory.java (revision 1512797) +++ lucene/core/src/test/org/apache/lucene/store/TestDirectory.java (working copy) @@ -134,52 +134,66 @@ // Test that different instances of FSDirectory can coexist on the same // path, can read, write, and lock files. public void testDirectInstantiation() throws Exception { - File path = _TestUtil.getTempDir("testDirectInstantiation"); + final File path = _TestUtil.getTempDir("testDirectInstantiation"); + + final byte[] largeBuffer = new byte[random().nextInt(256*1024)], largeReadBuffer = new byte[largeBuffer.length]; + for (int i = 0; i < largeBuffer.length; i++) { + largeBuffer[i] = (byte) i; // automatically loops with modulo + } - int sz = 3; - Directory[] dirs = new Directory[sz]; + final FSDirectory[] dirs = new FSDirectory[] { + new SimpleFSDirectory(path, null), + new NIOFSDirectory(path, null), + new MMapDirectory(path, null) + }; - dirs[0] = new SimpleFSDirectory(path, null); - dirs[1] = new NIOFSDirectory(path, null); - dirs[2] = new MMapDirectory(path, null); - - for (int i=0; i