Index: lucene/src/java/org/apache/lucene/store/MMapDirectory.java =================================================================== --- lucene/src/java/org/apache/lucene/store/MMapDirectory.java (revision 1135318) +++ lucene/src/java/org/apache/lucene/store/MMapDirectory.java (working copy) @@ -79,8 +79,8 @@ */ public class MMapDirectory extends FSDirectory { private boolean useUnmapHack = UNMAP_SUPPORTED; - public static final int DEFAULT_MAX_BUFF = Constants.JRE_IS_64BIT ? Integer.MAX_VALUE : (256 * 1024 * 1024); - private int maxBBuf = DEFAULT_MAX_BUFF; + public static final int DEFAULT_MAX_BUFF = Constants.JRE_IS_64BIT ? (1 << 30) : (1 << 28); + private int power; /** Create a new MMapDirectory for the named location. * @@ -91,6 +91,7 @@ */ public MMapDirectory(File path, LockFactory lockFactory) throws IOException { super(path, lockFactory); + setMaxChunkSize(DEFAULT_MAX_BUFF); } /** Create a new MMapDirectory for the named location and {@link NativeFSLockFactory}. @@ -100,6 +101,7 @@ */ public MMapDirectory(File path) throws IOException { super(path, null); + setMaxChunkSize(DEFAULT_MAX_BUFF); } /** @@ -180,23 +182,27 @@ * Especially on 32 bit platform, the address space can be very fragmented, * so large index files cannot be mapped. * Using a lower chunk size makes the directory implementation a little - * bit slower (as the correct chunk must be resolved on each seek) + * bit slower (as the correct chunk may be resolved on lots of seeks) * but the chance is higher that mmap does not fail. On 64 bit - * Java platforms, this parameter should always be {@link Integer#MAX_VALUE}, + * Java platforms, this parameter should always be {@code 1 << 30}, * as the address space is big enough. + * Please note: This method always rounds down the chunk size + * to a power of 2. */ - public void setMaxChunkSize(final int maxBBuf) { + public final void setMaxChunkSize(final int maxBBuf) { if (maxBBuf<=0) throw new IllegalArgumentException("Maximum chunk size for mmap must be >0"); - this.maxBBuf=maxBBuf; + //System.out.println("Requested buf size: "+maxBBuf); + this.power = 31 - Integer.numberOfLeadingZeros(maxBBuf); + //System.out.println("Got buf size: "+getMaxChunkSize()); } /** * Returns the current mmap chunk size. * @see #setMaxChunkSize */ - public int getMaxChunkSize() { - return maxBBuf; + public final int getMaxChunkSize() { + return 1 << power; } /** Creates an IndexInput for the file with the given name. */ @@ -206,149 +212,54 @@ File f = new File(getDirectory(), name); RandomAccessFile raf = new RandomAccessFile(f, "r"); try { - return (raf.length() <= maxBBuf) - ? (IndexInput) new MMapIndexInput(raf) - : (IndexInput) new MultiMMapIndexInput(raf, maxBBuf); + return new MMapIndexInput(raf, power); } finally { raf.close(); } } - private class MMapIndexInput extends IndexInput { - - private ByteBuffer buffer; - private final long length; - private boolean isClone = false; - - private MMapIndexInput(RandomAccessFile raf) throws IOException { - this.length = raf.length(); - this.buffer = raf.getChannel().map(MapMode.READ_ONLY, 0, length); - } - - @Override - public byte readByte() throws IOException { - try { - return buffer.get(); - } catch (BufferUnderflowException e) { - throw new IOException("read past EOF"); - } - } - - @Override - public void readBytes(byte[] b, int offset, int len) throws IOException { - try { - buffer.get(b, offset, len); - } catch (BufferUnderflowException e) { - throw new IOException("read past EOF"); - } - } - - @Override - public short readShort() throws IOException { - try { - return buffer.getShort(); - } catch (BufferUnderflowException e) { - throw new IOException("read past EOF"); - } - } - - @Override - public int readInt() throws IOException { - try { - return buffer.getInt(); - } catch (BufferUnderflowException e) { - throw new IOException("read past EOF"); - } - } - - @Override - public long readLong() throws IOException { - try { - return buffer.getLong(); - } catch (BufferUnderflowException e) { - throw new IOException("read past EOF"); - } - } - - @Override - public long getFilePointer() { - return buffer.position(); - } - - @Override - public void seek(long pos) throws IOException { - buffer.position((int)pos); - } - - @Override - public long length() { - return length; - } - - @Override - public Object clone() { - if (buffer == null) - throw new AlreadyClosedException("MMapIndexInput already closed"); - MMapIndexInput clone = (MMapIndexInput)super.clone(); - clone.isClone = true; - clone.buffer = buffer.duplicate(); - return clone; - } - - @Override - public void close() throws IOException { - // unmap the buffer (if enabled) and at least unset it for GC - try { - if (isClone || buffer == null) return; - cleanMapping(buffer); - } finally { - buffer = null; - } - } - } - // Because Java's ByteBuffer uses an int to address the // values, it's necessary to access a file > // Integer.MAX_VALUE in size using multiple byte buffers. - private class MultiMMapIndexInput extends IndexInput { + private final class MMapIndexInput extends IndexInput { private ByteBuffer[] buffers; - private final long length; + private final long length, mask, maxBufSize; + private final int power; private int curBufIndex; - private final int maxBufSize; private ByteBuffer curBuf; // redundant for speed: buffers[curBufIndex] private boolean isClone = false; - public MultiMMapIndexInput(RandomAccessFile raf, int maxBufSize) - throws IOException { + MMapIndexInput(RandomAccessFile raf, int power) throws IOException { this.length = raf.length(); - this.maxBufSize = maxBufSize; + this.power = power; + this.maxBufSize = 1L << power; + this.mask = maxBufSize - 1L; - if (maxBufSize <= 0) - throw new IllegalArgumentException("Non positive maxBufSize: " - + maxBufSize); + if (power < 0 || power > 30) + throw new IllegalArgumentException("Invalid power for buffer size: " + power); - if ((length / maxBufSize) > Integer.MAX_VALUE) - throw new IllegalArgumentException - ("RandomAccessFile too big for maximum buffer size: " - + raf.toString()); + if ((length >>> power) >= Integer.MAX_VALUE) + throw new IllegalArgumentException("RandomAccessFile too big for maximum buffer size: " + raf.toString()); - int nrBuffers = (int) (length / maxBufSize); - if (((long) nrBuffers * maxBufSize) <= length) nrBuffers++; + // we always allocate one more buffer, the last one may be a 0 byte one + final int nrBuffers = (int) (length >>> power) + 1; + //System.out.println("Length="+length+", Power=" + power + ", Mask=" + mask + ", nrBuffers=" + nrBuffers); + this.buffers = new ByteBuffer[nrBuffers]; - long bufferStart = 0; + long bufferStart = 0L; FileChannel rafc = raf.getChannel(); for (int bufNr = 0; bufNr < nrBuffers; bufNr++) { int bufSize = (length > (bufferStart + maxBufSize)) - ? maxBufSize + ? (int) maxBufSize : (int) (length - bufferStart); - this.buffers[bufNr] = rafc.map(MapMode.READ_ONLY,bufferStart,bufSize); + this.buffers[bufNr] = rafc.map(MapMode.READ_ONLY, bufferStart, bufSize); bufferStart += bufSize; } seek(0L); @@ -364,6 +275,8 @@ throw new IOException("read past EOF"); curBuf = buffers[curBufIndex]; curBuf.position(0); + if (!curBuf.hasRemaining()) + throw new IOException("read past EOF"); return curBuf.get(); } } @@ -418,15 +331,28 @@ @Override public long getFilePointer() { - return ((long) curBufIndex * maxBufSize) + curBuf.position(); + return (((long) curBufIndex) << power) + curBuf.position(); } @Override public void seek(long pos) throws IOException { - curBufIndex = (int) (pos / maxBufSize); - curBuf = buffers[curBufIndex]; - int bufOffset = (int) (pos - ((long) curBufIndex * maxBufSize)); - curBuf.position(bufOffset); + // we use >> here to preserve negative, so we will catch AIOOBE: + final int bi = (int) (pos >> power); + try { + final ByteBuffer b = buffers[bi]; + b.position((int) (pos & mask)); + // write values, on exception all is unchanged + this.curBufIndex = bi; + this.curBuf = b; + } catch (ArrayIndexOutOfBoundsException aioobe) { + if (pos < 0L) + throw new IllegalArgumentException("Seeking to negative position"); + throw new IOException("seek past EOF"); + } catch (IllegalArgumentException iae) { + if (pos < 0L) + throw new IllegalArgumentException("Seeking to negative position"); + throw new IOException("seek past EOF"); + } } @Override @@ -437,8 +363,8 @@ @Override public Object clone() { if (buffers == null) - throw new AlreadyClosedException("MultiMMapIndexInput already closed"); - MultiMMapIndexInput clone = (MultiMMapIndexInput)super.clone(); + throw new AlreadyClosedException("MMapIndexInput already closed"); + final MMapIndexInput clone = (MMapIndexInput)super.clone(); clone.isClone = true; clone.buffers = new ByteBuffer[buffers.length]; // Since most clones will use only one buffer, duplicate() could also be @@ -449,9 +375,7 @@ try { clone.seek(getFilePointer()); } catch(IOException ioe) { - RuntimeException newException = new RuntimeException(ioe); - newException.initCause(ioe); - throw newException; + throw new RuntimeException("Should never happen", ioe); } return clone; }