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/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 @@ -178,10 +179,6 @@ 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 +186,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) @@ -85,24 +85,9 @@ 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 { Index: lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java =================================================================== --- lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (revision 1512797) +++ lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (working copy) @@ -1139,7 +1139,6 @@ } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException e) { Rethrow.rethrow(e); } - d.setReadChunkSize(_TestUtil.nextInt(random(), 8, 32678)); return d; }