ReceivePackStats: Add size and count of unnecessary pushed objects

Since there is no negotiation for a push, the client is probably sending
redundant objects and bytes which already exist in the server.

Add more metrics in the stats to quantify it. Duplicated size and number
to measure the size and the number of duplicated objects which should
not be pushed.

Change-Id: Iaacd4761ee9366a0a7ec4e26c508eff45c8744de
Signed-off-by: Yunjie Li <yunjieli@google.com>

4 files changed, 137 insertions, 5 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
index e9b4af932e..945900f14b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
@@ -1060,6 +1060,67 @@ public class UploadPackTest {
 	}
 
 	@Test
+	public void testUploadNewBytes() throws Exception {
+		String commonInBlob = "abcdefghijklmnopqrstuvwx";
+
+		RevBlob parentBlob = remote.blob(commonInBlob + "a");
+		RevCommit parent = remote.commit(remote.tree(remote.file("foo", parentBlob)));
+		RevBlob childBlob = remote.blob(commonInBlob + "b");
+		RevCommit child = remote.commit(remote.tree(remote.file("foo", childBlob)), parent);
+		remote.update("branch1", child);
+
+		ByteArrayInputStream recvStream = uploadPackV2(
+			"command=fetch\n",
+			PacketLineIn.delimiter(),
+			"want " + child.toObjectId().getName() + "\n",
+			"ofs-delta\n",
+			"done\n",
+				PacketLineIn.end());
+		PacketLineIn pckIn = new PacketLineIn(recvStream);
+		assertThat(pckIn.readString(), is("packfile"));
+		ReceivedPackStatistics receivedStats = parsePack(recvStream);
+		assertTrue(receivedStats.getNumBytesDuplicated() == 0);
+		assertTrue(receivedStats.getNumObjectsDuplicated() == 0);
+	}
+
+	@Test
+	public void testUploadRedundantBytes() throws Exception {
+		String commonInBlob = "abcdefghijklmnopqrstuvwxyz";
+
+		RevBlob parentBlob = remote.blob(commonInBlob + "a");
+		RevCommit parent = remote.commit(remote.tree(remote.file("foo", parentBlob)));
+		RevBlob childBlob = remote.blob(commonInBlob + "b");
+		RevCommit child = remote.commit(remote.tree(remote.file("foo", childBlob)), parent);
+		remote.update("branch1", child);
+
+		TestRepository<InMemoryRepository> local = new TestRepository<>(client);
+		RevBlob localParentBlob = local.blob(commonInBlob + "a");
+		RevCommit localParent = local.commit(local.tree(local.file("foo", localParentBlob)));
+		RevBlob localChildBlob = local.blob(commonInBlob + "b");
+		RevCommit localChild = local.commit(
+				local.tree(local.file("foo", localChildBlob)), localParent);
+		local.update("branch1", localChild);
+
+		ByteArrayInputStream recvStream = uploadPackV2(
+				"command=fetch\n",
+				PacketLineIn.delimiter(),
+				"want " + child.toObjectId().getName() + "\n",
+				"ofs-delta\n",
+				"done\n",
+					PacketLineIn.end());
+		PacketLineIn pckIn = new PacketLineIn(recvStream);
+		assertThat(pckIn.readString(), is("packfile"));
+		ReceivedPackStatistics receivedStats = parsePack(recvStream);
+
+		long sizeOfHeader = 12;
+		long sizeOfTrailer = 20;
+		long expectedSize = receivedStats.getNumBytesRead() - sizeOfHeader
+				- sizeOfTrailer;
+		assertTrue(receivedStats.getNumBytesDuplicated() == expectedSize);
+		assertTrue(receivedStats.getNumObjectsDuplicated() == 6);
+	}
+
+	@Test
 	public void testV2FetchOfsDelta() throws Exception {
 		String commonInBlob = "abcdefghijklmnopqrstuvwxyz";
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
index 0801b8a86a..715cbb48fb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java
@@ -679,7 +679,8 @@ public abstract class PackParser {
 
 			verifySafeObject(tempObjectId, type, visit.data);
 			if (isCheckObjectCollisions() && readCurs.has(tempObjectId)) {
-				checkObjectCollision(tempObjectId, type, visit.data);
+				checkObjectCollision(tempObjectId, type, visit.data,
+						visit.delta.sizeBeforeInflating);
 			}
 
 			PackedObjectInfo oe;
@@ -999,6 +1000,7 @@ public abstract class PackParser {
 			UnresolvedDelta n = onEndDelta();
 			n.position = streamPosition;
 			n.next = baseByPos.put(base, n);
+			n.sizeBeforeInflating = streamPosition() - streamPosition;
 			deltaCount++;
 			break;
 		}
@@ -1020,6 +1022,7 @@ public abstract class PackParser {
 			inflateAndSkip(Source.INPUT, sz);
 			UnresolvedDelta n = onEndDelta();
 			n.position = streamPosition;
+			n.sizeBeforeInflating = streamPosition() - streamPosition;
 			r.add(n);
 			deltaCount++;
 			break;
@@ -1071,9 +1074,11 @@ public abstract class PackParser {
 			verifySafeObject(tempObjectId, type, data);
 		}
 
+		long sizeBeforeInflating = streamPosition() - pos;
 		PackedObjectInfo obj = newInfo(tempObjectId, null, null);
 		obj.setOffset(pos);
 		obj.setType(type);
+		obj.setSize(sizeBeforeInflating);
 		onEndWholeObject(obj);
 		if (data != null)
 			onInflatedObjectData(obj, type, data);
@@ -1148,6 +1153,8 @@ public abstract class PackParser {
 					sz -= n;
 				}
 			}
+			stats.incrementObjectsDuplicated();
+			stats.incrementNumBytesDuplicated(obj.getSize());
 		} catch (MissingObjectException notLocal) {
 			// This is OK, we don't have a copy of the object locally
 			// but the API throws when we try to read it as usually it's
@@ -1155,15 +1162,17 @@ public abstract class PackParser {
 		}
 	}
 
-	private void checkObjectCollision(AnyObjectId obj, int type, byte[] data)
-			throws IOException {
+	private void checkObjectCollision(AnyObjectId obj, int type, byte[] data,
+			long sizeBeforeInflating) throws IOException {
 		try {
 			final ObjectLoader ldr = readCurs.open(obj, type);
 			final byte[] existingData = ldr.getCachedBytes(data.length);
 			if (!Arrays.equals(data, existingData)) {
-				throw new IOException(MessageFormat.format(
-						JGitText.get().collisionOn, obj.name()));
+				throw new IOException(MessageFormat
+						.format(JGitText.get().collisionOn, obj.name()));
 			}
+			stats.incrementObjectsDuplicated();
+			stats.incrementNumBytesDuplicated(sizeBeforeInflating);
 		} catch (MissingObjectException notLocal) {
 			// This is OK, we don't have a copy of the object locally
 			// but the API throws when we try to read it as usually its
@@ -1653,6 +1662,8 @@ public abstract class PackParser {
 
 		UnresolvedDelta next;
 
+		long sizeBeforeInflating;
+
 		/** @return offset within the input stream. */
 		public long getOffset() {
 			return position;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackedObjectInfo.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackedObjectInfo.java
index fc906de2a8..fe1209b6af 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackedObjectInfo.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackedObjectInfo.java
@@ -29,6 +29,8 @@ public class PackedObjectInfo extends ObjectIdOwnerMap.Entry {
 
 	private int type = Constants.OBJ_BAD;
 
+	private long sizeBeforeInflating;
+
 	PackedObjectInfo(final long headerOffset, final int packedCRC,
 			final AnyObjectId id) {
 		super(id);
@@ -108,4 +110,12 @@ public class PackedObjectInfo extends ObjectIdOwnerMap.Entry {
 	public void setType(int type) {
 		this.type = type;
 	}
+
+	void setSize(long sizeBeforeInflating) {
+		this.sizeBeforeInflating = sizeBeforeInflating;
+	}
+
+	long getSize() {
+		return sizeBeforeInflating;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivedPackStatistics.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivedPackStatistics.java
index bd8f5585d2..d7bc40006b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivedPackStatistics.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivedPackStatistics.java
@@ -19,6 +19,7 @@ import org.eclipse.jgit.lib.Constants;
  */
 public class ReceivedPackStatistics {
 	private long numBytesRead;
+	private long numBytesDuplicated;
 
 	private long numWholeCommit;
 	private long numWholeTree;
@@ -26,6 +27,7 @@ public class ReceivedPackStatistics {
 	private long numWholeTag;
 	private long numOfsDelta;
 	private long numRefDelta;
+	private long numObjectsDuplicated;
 
 	private long numDeltaCommit;
 	private long numDeltaTree;
@@ -42,6 +44,17 @@ public class ReceivedPackStatistics {
 	}
 
 	/**
+	 * Get number of bytes of objects already in the local database
+	 *
+	 * @return number of bytes of objects appeared in both the pack sent by the
+	 *         client and the local database
+	 * @since 5.10
+	 */
+	public long getNumBytesDuplicated() {
+		return numBytesDuplicated;
+	}
+
+	/**
 	 * Get number of whole commit objects in the pack
 	 *
 	 * @return number of whole commit objects in the pack
@@ -96,6 +109,17 @@ public class ReceivedPackStatistics {
 	}
 
 	/**
+	 * Get number of objects already in the local database
+	 *
+	 * @return number of objects appeared in both the pack sent by the client
+	 *         and the local database
+	 * @since 5.10
+	 */
+	public long getNumObjectsDuplicated() {
+		return numObjectsDuplicated;
+	}
+
+	/**
 	 * Get number of delta commit objects in the pack
 	 *
 	 * @return number of delta commit objects in the pack
@@ -134,6 +158,7 @@ public class ReceivedPackStatistics {
 	/** A builder for {@link ReceivedPackStatistics}. */
 	public static class Builder {
 		private long numBytesRead;
+		private long numBytesDuplicated;
 
 		private long numWholeCommit;
 		private long numWholeTree;
@@ -141,6 +166,7 @@ public class ReceivedPackStatistics {
 		private long numWholeTag;
 		private long numOfsDelta;
 		private long numRefDelta;
+		private long numObjectsDuplicated;
 
 		private long numDeltaCommit;
 		private long numDeltaTree;
@@ -157,6 +183,17 @@ public class ReceivedPackStatistics {
 		}
 
 		/**
+		 * @param size
+		 *            additional bytes already in the local database
+		 * @return this
+		 * @since 5.10
+		 */
+		Builder incrementNumBytesDuplicated(long size) {
+			numBytesDuplicated += size;
+			return this;
+		}
+
+		/**
 		 * Increment a whole object count.
 		 *
 		 * @param type OBJ_COMMIT, OBJ_TREE, OBJ_BLOB, or OBJ_TAG
@@ -196,6 +233,17 @@ public class ReceivedPackStatistics {
 		}
 
 		/**
+		 * Increment the duplicated object count.
+		 *
+		 * @return this
+		 * @since 5.10
+		 */
+		Builder incrementObjectsDuplicated() {
+			numObjectsDuplicated++;
+			return this;
+		}
+
+		/**
 		 * Increment a delta object count.
 		 *
 		 * @param type OBJ_COMMIT, OBJ_TREE, OBJ_BLOB, or OBJ_TAG
@@ -226,6 +274,7 @@ public class ReceivedPackStatistics {
 		ReceivedPackStatistics build() {
 			ReceivedPackStatistics s = new ReceivedPackStatistics();
 			s.numBytesRead = numBytesRead;
+			s.numBytesDuplicated = numBytesDuplicated;
 			s.numWholeCommit = numWholeCommit;
 			s.numWholeTree = numWholeTree;
 			s.numWholeBlob = numWholeBlob;
@@ -236,6 +285,7 @@ public class ReceivedPackStatistics {
 			s.numDeltaTree = numDeltaTree;
 			s.numDeltaBlob = numDeltaBlob;
 			s.numDeltaTag = numDeltaTag;
+			s.numObjectsDuplicated = numObjectsDuplicated;
 			return s;
 		}
 	}