From 58e991b5dee510ea372716b054b78994df4d63c0 Mon Sep 17 00:00:00 2001 From: Yunjie Li Date: Wed, 25 Mar 2020 10:59:06 -0700 Subject: [PATCH] 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 --- .../jgit/transport/UploadPackTest.java | 61 +++++++++++++++++++ .../eclipse/jgit/transport/PackParser.java | 21 +++++-- .../jgit/transport/PackedObjectInfo.java | 10 +++ .../transport/ReceivedPackStatistics.java | 50 +++++++++++++++ 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 e9b4af932..945900f14 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 @@ -1059,6 +1059,67 @@ public void testV2FetchIncludeTag() throws Exception { assertTrue(client.getObjectDatabase().has(tag.toObjectId())); } + @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 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 0801b8a86..715cbb48f 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 @@ private void resolveDeltas(DeltaVisit visit, final int type, 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 @@ private void indexOneObject() throws IOException { UnresolvedDelta n = onEndDelta(); n.position = streamPosition; n.next = baseByPos.put(base, n); + n.sizeBeforeInflating = streamPosition() - streamPosition; deltaCount++; break; } @@ -1020,6 +1022,7 @@ private void indexOneObject() throws IOException { inflateAndSkip(Source.INPUT, sz); UnresolvedDelta n = onEndDelta(); n.position = streamPosition; + n.sizeBeforeInflating = streamPosition() - streamPosition; r.add(n); deltaCount++; break; @@ -1071,9 +1074,11 @@ private void whole(long pos, int type, long sz) 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 @@ private void checkObjectCollision(PackedObjectInfo obj) 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 @@ private void checkObjectCollision(PackedObjectInfo obj) } } - 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 static class UnresolvedDelta { 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 fc906de2a..fe1209b6a 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 int getType() { 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 bd8f5585d..d7bc40006 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 @@ */ 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; @@ -41,6 +43,17 @@ public long getNumBytesRead() { return numBytesRead; } + /** + * 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 * @@ -95,6 +108,17 @@ public long getNumRefDelta() { return numRefDelta; } + /** + * 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 * @@ -134,6 +158,7 @@ public long getNumDeltaTag() { /** 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 static class Builder { private long numWholeTag; private long numOfsDelta; private long numRefDelta; + private long numObjectsDuplicated; private long numDeltaCommit; private long numDeltaTree; @@ -156,6 +182,17 @@ public Builder setNumBytesRead(long numBytesRead) { return this; } + /** + * @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. * @@ -195,6 +232,17 @@ public Builder addRefDelta() { return this; } + /** + * Increment the duplicated object count. + * + * @return this + * @since 5.10 + */ + Builder incrementObjectsDuplicated() { + numObjectsDuplicated++; + return this; + } + /** * Increment a delta object count. * @@ -226,6 +274,7 @@ public Builder addDeltaObject(int type) { 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 @@ ReceivedPackStatistics build() { s.numDeltaTree = numDeltaTree; s.numDeltaBlob = numDeltaBlob; s.numDeltaTag = numDeltaTag; + s.numObjectsDuplicated = numObjectsDuplicated; return s; } }