changeset 158:b413b16d10a5

Integer offsets and file length explictly, rather than casts throughout code. Inflater may benefit from total length hint, but shall calculate it by its own if needed
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Wed, 09 Mar 2011 13:16:37 +0100
parents d5268ca7715b
children 7a8af814908b
files cmdline/org/tmatesoft/hg/console/Main.java src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java src/org/tmatesoft/hg/internal/DataAccess.java src/org/tmatesoft/hg/internal/DataAccessProvider.java src/org/tmatesoft/hg/internal/FilterDataAccess.java src/org/tmatesoft/hg/internal/InflaterDataAccess.java src/org/tmatesoft/hg/internal/RevlogStream.java src/org/tmatesoft/hg/repo/HgDataFile.java
diffstat 8 files changed, 91 insertions(+), 51 deletions(-) [+]
line wrap: on
line diff
--- a/cmdline/org/tmatesoft/hg/console/Main.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/cmdline/org/tmatesoft/hg/console/Main.java	Wed Mar 09 13:16:37 2011 +0100
@@ -76,7 +76,9 @@
 		System.out.println(f1.isCopy());
 		System.out.println(f2.isCopy());
 		ByteArrayChannel bac = new ByteArrayChannel();
-		f1.content(0, bac);
+		f1.content(1, bac); // 0: 1151, 1: 1139
+		System.out.println(bac.toArray().length);
+		f2.content(0, bac = new ByteArrayChannel()); // 0: 14269
 		System.out.println(bac.toArray().length);
 	}
 	
--- a/src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/ByteArrayDataAccess.java	Wed Mar 09 13:16:37 2011 +0100
@@ -64,11 +64,11 @@
 		return this;
 	}
 	@Override
-	public long length() {
+	public int length() {
 		return length;
 	}
 	@Override
-	public void seek(long offset) {
+	public void seek(int offset) {
 		pos = (int) offset;
 	}
 	@Override
--- a/src/org/tmatesoft/hg/internal/DataAccess.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/DataAccess.java	Wed Mar 09 13:16:37 2011 +0100
@@ -32,7 +32,7 @@
 	public boolean isEmpty() {
 		return true;
 	}
-	public long length() {
+	public int length() {
 		return 0;
 	}
 	/**
@@ -45,7 +45,7 @@
 		return this;
 	}
 	// absolute positioning
-	public void seek(long offset) throws IOException {
+	public void seek(int offset) throws IOException {
 		throw new UnsupportedOperationException();
 	}
 	// relative positioning
@@ -95,7 +95,7 @@
 	// FIXME exception handling is not right, just for the sake of quick test
 	public byte[] byteArray() throws IOException {
 		reset();
-		byte[] rv = new byte[(int) length()];
+		byte[] rv = new byte[length()];
 		readBytes(rv, 0, rv.length);
 		return rv;
 	}
--- a/src/org/tmatesoft/hg/internal/DataAccessProvider.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/DataAccessProvider.java	Wed Mar 09 13:16:37 2011 +0100
@@ -23,6 +23,8 @@
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel;
 
+import org.tmatesoft.hg.core.HgBadStateException;
+
 /**
  * 
  * @author Artem Tikhomirov
@@ -48,16 +50,20 @@
 		}
 		try {
 			FileChannel fc = new FileInputStream(f).getChannel();
-			if (fc.size() > mapioMagicBoundary) {
+			int flen = (int) fc.size();
+			if (fc.size() - flen != 0) {
+				throw new HgBadStateException("Files greater than 2Gb are not yet supported");
+			}
+			if (flen > mapioMagicBoundary) {
 				// TESTS: bufLen of 1024 was used to test MemMapFileAccess
-				return new MemoryMapFileAccess(fc, fc.size(), mapioMagicBoundary);
+				return new MemoryMapFileAccess(fc, flen, mapioMagicBoundary);
 			} else {
 				// XXX once implementation is more or less stable,
 				// may want to try ByteBuffer.allocateDirect() to see
 				// if there's any performance gain. 
 				boolean useDirectBuffer = false;
-				// TESTS: bufferSize of 100 was used to check buffer underflow states when readBytes reads chunks bigger than bufSize 
-				return new FileAccess(fc, fc.size(), bufferSize, useDirectBuffer);
+				// TESTS: bufferSize of 100 was used to check buffer underflow states when readBytes reads chunks bigger than bufSize
+				return new FileAccess(fc, flen, bufferSize, useDirectBuffer);
 			}
 		} catch (IOException ex) {
 			// unlikely to happen, we've made sure file exists.
@@ -69,12 +75,12 @@
 	// DOESN'T WORK YET 
 	private static class MemoryMapFileAccess extends DataAccess {
 		private FileChannel fileChannel;
-		private final long size;
+		private final int size;
 		private long position = 0; // always points to buffer's absolute position in the file
 		private final int memBufferSize;
 		private MappedByteBuffer buffer;
 
-		public MemoryMapFileAccess(FileChannel fc, long channelSize, int /*long?*/ bufferSize) {
+		public MemoryMapFileAccess(FileChannel fc, int channelSize, int /*long?*/ bufferSize) {
 			fileChannel = fc;
 			size = channelSize;
 			memBufferSize = bufferSize;
@@ -86,7 +92,7 @@
 		}
 		
 		@Override
-		public long length() {
+		public int length() {
 			return size;
 		}
 		
@@ -97,7 +103,7 @@
 		}
 		
 		@Override
-		public void seek(long offset) {
+		public void seek(int offset) {
 			assert offset >= 0;
 			// offset may not necessarily be further than current position in the file (e.g. rewind) 
 			if (buffer != null && /*offset is within buffer*/ offset >= position && (offset - position) < buffer.limit()) {
@@ -181,14 +187,14 @@
 	// (almost) regular file access - FileChannel and buffers.
 	private static class FileAccess extends DataAccess {
 		private FileChannel fileChannel;
-		private final long size;
+		private final int size;
 		private ByteBuffer buffer;
-		private long bufferStartInFile = 0; // offset of this.buffer in the file.
+		private int bufferStartInFile = 0; // offset of this.buffer in the file.
 
-		public FileAccess(FileChannel fc, long channelSize, int bufferSizeHint, boolean useDirect) {
+		public FileAccess(FileChannel fc, int channelSize, int bufferSizeHint, boolean useDirect) {
 			fileChannel = fc;
 			size = channelSize;
-			final int capacity = size < bufferSizeHint ? (int) size : bufferSizeHint;
+			final int capacity = size < bufferSizeHint ? size : bufferSizeHint;
 			buffer = useDirect ? ByteBuffer.allocateDirect(capacity) : ByteBuffer.allocate(capacity);
 			buffer.flip(); // or .limit(0) to indicate it's empty
 		}
@@ -199,7 +205,7 @@
 		}
 		
 		@Override
-		public long length() {
+		public int length() {
 			return size;
 		}
 		
@@ -210,7 +216,7 @@
 		}
 		
 		@Override
-		public void seek(long offset) throws IOException {
+		public void seek(int offset) throws IOException {
 			if (offset > size) {
 				throw new IllegalArgumentException();
 			}
--- a/src/org/tmatesoft/hg/internal/FilterDataAccess.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/FilterDataAccess.java	Wed Mar 09 13:16:37 2011 +0100
@@ -28,11 +28,11 @@
  */
 public class FilterDataAccess extends DataAccess {
 	private final DataAccess dataAccess;
-	private final long offset;
+	private final int offset;
 	private final int length;
 	private int count;
 
-	public FilterDataAccess(DataAccess dataAccess, long offset, int length) {
+	public FilterDataAccess(DataAccess dataAccess, int offset, int length) {
 		this.dataAccess = dataAccess;
 		this.offset = offset;
 		this.length = length;
@@ -55,12 +55,12 @@
 	}
 	
 	@Override
-	public long length() {
+	public int length() {
 		return length;
 	}
 
 	@Override
-	public void seek(long localOffset) throws IOException {
+	public void seek(int localOffset) throws IOException {
 		if (localOffset < 0 || localOffset > length) {
 			throw new IllegalArgumentException();
 		}
--- a/src/org/tmatesoft/hg/internal/InflaterDataAccess.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/InflaterDataAccess.java	Wed Mar 09 13:16:37 2011 +0100
@@ -22,6 +22,8 @@
 import java.util.zip.Inflater;
 import java.util.zip.ZipException;
 
+import org.tmatesoft.hg.core.HgBadStateException;
+
 
 /**
  * DataAccess counterpart for InflaterInputStream.
@@ -36,15 +38,20 @@
 	private final byte[] buffer;
 	private final byte[] singleByte = new byte[1];
 	private int decompressedPos = 0;
-	private int decompressedLength = -1;
+	private int decompressedLength;
 
-	public InflaterDataAccess(DataAccess dataAccess, long offset, int length) {
-		this(dataAccess, offset, length, new Inflater(), 512);
+	public InflaterDataAccess(DataAccess dataAccess, int offset, int compressedLength) {
+		this(dataAccess, offset, compressedLength, -1, new Inflater(), 512);
 	}
 
-	public InflaterDataAccess(DataAccess dataAccess, long offset, int length, Inflater inflater, int bufSize) {
-		super(dataAccess, offset, length);
+	public InflaterDataAccess(DataAccess dataAccess, int offset, int compressedLength, int actualLength) {
+		this(dataAccess, offset, compressedLength, actualLength, new Inflater(), 512);
+	}
+
+	public InflaterDataAccess(DataAccess dataAccess, int offset, int compressedLength, int actualLength, Inflater inflater, int bufSize) {
+		super(dataAccess, offset, compressedLength);
 		this.inflater = inflater;
+		this.decompressedLength = actualLength;
 		buffer = new byte[bufSize];
 	}
 	
@@ -58,39 +65,53 @@
 	
 	@Override
 	protected int available() {
-		throw new IllegalStateException("Can't tell how much uncompressed data left");
+		return length() - decompressedPos;
 	}
 	
 	@Override
 	public boolean isEmpty() {
-		return super.available() <= 0 && inflater.finished(); // and/or inflater.getRemaining() <= 0 ?
+		// can't use super.available() <= 0 because even when 0 < super.count < 6(?)
+		// decompressedPos might be already == length() 
+		return available() <= 0;
 	}
 	
 	@Override
-	public long length() {
+	public int length() {
 		if (decompressedLength != -1) {
 			return decompressedLength;
 		}
+		decompressedLength = 0; // guard to avoid endless loop in case length() would get invoked from below. 
 		int c = 0;
 		try {
 			int oldPos = decompressedPos;
-			while (!isEmpty()) {
-				readByte();
-				c++;
+			byte[] dummy = new byte[buffer.length];
+			int toRead;
+			while ((toRead = super.available()) > 0) {
+				if (toRead > buffer.length) {
+					toRead = buffer.length;
+				}
+				super.readBytes(buffer, 0, toRead);
+				inflater.setInput(buffer, 0, toRead);
+				try {
+					while (!inflater.needsInput()) {
+						c += inflater.inflate(dummy, 0, dummy.length);
+					}
+				} catch (DataFormatException ex) {
+					throw new HgBadStateException(ex);
+				}
 			}
 			decompressedLength = c + oldPos;
 			reset();
 			seek(oldPos);
 			return decompressedLength;
 		} catch (IOException ex) {
-			ex.printStackTrace(); // FIXME log error
 			decompressedLength = -1; // better luck next time?
-			return 0;
+			throw new HgBadStateException(ex); // XXX perhaps, checked exception
 		}
 	}
 	
 	@Override
-	public void seek(long localOffset) throws IOException {
+	public void seek(int localOffset) throws IOException {
 		if (localOffset < 0 /* || localOffset >= length() */) {
 			throw new IllegalArgumentException();
 		}
--- a/src/org/tmatesoft/hg/internal/RevlogStream.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/internal/RevlogStream.java	Wed Mar 09 13:16:37 2011 +0100
@@ -26,6 +26,7 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import org.tmatesoft.hg.core.HgBadStateException;
 import org.tmatesoft.hg.core.Nodeid;
 import org.tmatesoft.hg.repo.HgRepository;
 
@@ -77,7 +78,7 @@
 			revision = indexSize - 1;
 		}
 		try {
-			int recordOffset = inline ? (int) index.get(revision).offset : revision * REVLOGV1_RECORD_SIZE;
+			int recordOffset = inline ? index.get(revision).getIntOffset() : revision * REVLOGV1_RECORD_SIZE;
 			daIndex.seek(recordOffset + 12); // 6+2+4
 			int actualLen = daIndex.readInt();
 			return actualLen; 
@@ -99,7 +100,7 @@
 		}
 		DataAccess daIndex = getIndexStream();
 		try {
-			int recordOffset = inline ? (int) index.get(revision).offset : revision * REVLOGV1_RECORD_SIZE;
+			int recordOffset = inline ? index.get(revision).getIntOffset() : revision * REVLOGV1_RECORD_SIZE;
 			daIndex.seek(recordOffset + 32);
 			byte[] rv = new byte[20];
 			daIndex.readBytes(rv, 0, 20);
@@ -122,7 +123,7 @@
 		}
 		DataAccess daIndex = getIndexStream();
 		try {
-			int recordOffset = inline ? (int) index.get(revision).offset : revision * REVLOGV1_RECORD_SIZE;
+			int recordOffset = inline ? index.get(revision).getIntOffset() : revision * REVLOGV1_RECORD_SIZE;
 			daIndex.seek(recordOffset + 20);
 			int linkRev = daIndex.readInt();
 			return linkRev;
@@ -208,11 +209,11 @@
 				i = start;
 			}
 			
-			daIndex.seek(inline ? index.get(i).offset : i * REVLOGV1_RECORD_SIZE);
+			daIndex.seek(inline ? index.get(i).getIntOffset() : i * REVLOGV1_RECORD_SIZE);
 			for (; i <= end; i++ ) {
 				if (inline && needData) {
 					// inspector reading data (though FilterDataAccess) may have affected index position
-					daIndex.seek(index.get(i).offset);
+					daIndex.seek(index.get(i).getIntOffset());
 				}
 				long l = daIndex.readLong(); // 0
 				@SuppressWarnings("unused")
@@ -231,7 +232,7 @@
 				DataAccess userDataAccess = null;
 				if (needData) {
 					final byte firstByte;
-					long streamOffset = index.get(i).offset;
+					int streamOffset = index.get(i).getIntOffset();
 					DataAccess streamDataAccess;
 					if (inline) {
 						streamDataAccess = daIndex;
@@ -240,9 +241,10 @@
 						streamDataAccess = daData;
 						daData.seek(streamOffset);
 					}
+					final boolean patchToPrevious = baseRevision != i; // XXX not sure if this is the right way to detect a patch
 					firstByte = streamDataAccess.readByte();
 					if (firstByte == 0x78 /* 'x' */) {
-						userDataAccess = new InflaterDataAccess(streamDataAccess, streamOffset, compressedLen);
+						userDataAccess = new InflaterDataAccess(streamDataAccess, streamOffset, compressedLen, patchToPrevious ? -1 : actualLen);
 					} else if (firstByte == 0x75 /* 'u' */) {
 						userDataAccess = new FilterDataAccess(streamDataAccess, streamOffset+1, compressedLen-1);
 					} else {
@@ -251,7 +253,7 @@
 						userDataAccess = new FilterDataAccess(streamDataAccess, streamOffset, compressedLen);
 					}
 					// XXX 
-					if (baseRevision != i) { // XXX not sure if this is the right way to detect a patch
+					if (patchToPrevious) {
 						// this is a patch
 						LinkedList<PatchRecord> patches = new LinkedList<PatchRecord>();
 						while (!userDataAccess.isEmpty()) {
@@ -281,7 +283,7 @@
 				}
 			}
 		} catch (IOException ex) {
-			throw new IllegalStateException(ex); // FIXME need better handling
+			throw new HgBadStateException(ex); // FIXME need better handling
 		} finally {
 			daIndex.done();
 			if (daData != null) {
@@ -345,14 +347,23 @@
 	// perhaps, package-local or protected, if anyone else from low-level needs them
 	// XXX think over if we should keep offset in case of separate data file - we read the field anyway. Perhaps, distinct entry classes for Inline and non-inline indexes?
 	private static class IndexEntry {
-		public final long offset; // for separate .i and .d - copy of index record entry, for inline index - actual offset of the record in the .i file (record entry + revision * record size))
+		private final int/*long*/ offset; // for separate .i and .d - copy of index record entry, for inline index - actual offset of the record in the .i file (record entry + revision * record size))
 		//public final int length; // data past fixed record (need to decide whether including header size or not), and whether length is of compressed data or not
 		public final int baseRevision;
 
 		public IndexEntry(long o, int baseRev) {
-			offset = o;
+			offset = (int) o;
+			// Index file stores offsets as long, but since DataAccess doesn't support long length() and others yet, 
+			// no reason to operate with long offsets 
+			if (o != offset) {
+				throw new HgBadStateException("Data too big, offset didn't fit to sizeof(int)");
+			}
 			baseRevision = baseRev;
 		}
+
+		public int getIntOffset() {
+			return offset;
+		}
 	}
 
 	// mpatch.c : apply()
@@ -360,7 +371,7 @@
 	public/*for HgBundle; until moved to better place*/static byte[] apply(DataAccess baseRevisionContent, int outcomeLen, List<PatchRecord> patch) throws IOException {
 		int last = 0, destIndex = 0;
 		if (outcomeLen == -1) {
-			outcomeLen = (int) baseRevisionContent.length();
+			outcomeLen = baseRevisionContent.length();
 			for (PatchRecord pr : patch) {
 				outcomeLen += pr.start - last + pr.len;
 				last = pr.end;
--- a/src/org/tmatesoft/hg/repo/HgDataFile.java	Wed Mar 09 05:22:17 2011 +0100
+++ b/src/org/tmatesoft/hg/repo/HgDataFile.java	Wed Mar 09 13:16:37 2011 +0100
@@ -320,7 +320,7 @@
 
 		@Override
 		protected void prepare(int revisionNumber, DataAccess da) throws HgException, IOException {
-			long daLength = da.length();
+			final int daLength = da.length();
 			if (daLength < 4 || da.readByte() != 1 || da.readByte() != 10) {
 				metadata.recordNone(revisionNumber);
 				da.reset();