]> source.dussan.org Git - jgit.git/commitdiff
Optimize RefAdvertiser for wire protocol 24/75524/1
authorShawn Pearce <spearce@spearce.org>
Sun, 19 Jun 2016 04:40:35 +0000 (21:40 -0700)
committerShawn Pearce <spearce@spearce.org>
Sun, 19 Jun 2016 04:41:07 +0000 (21:41 -0700)
The native wire protocol sends ref advertisements in the pkt-line
format, which requires encoding the ObjectId and ref name onto a byte
sequence.  Busy servers show this is a very high source of garbage,
which pushes the garbage collector harder when there are many refs in
the repository (e.g.  70k, in a Gerrit managed repository).

Optimize the side band advertiser by retaining the CharsetEncoder,
minimizing the amount of temporary garbage built during encoding.

Change-Id: I406c654bf82c1eb94b38862da2425e98396134cb

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefAdvertiserTest.java [new file with mode: 0644]
org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java
org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefAdvertiserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefAdvertiserTest.java
new file mode 100644 (file)
index 0000000..ce69adf
--- /dev/null
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2016, Google Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.transport;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
+import org.eclipse.jgit.util.NB;
+import org.junit.Test;
+
+public class RefAdvertiserTest {
+       @Test
+       public void advertiser() throws IOException {
+               ByteArrayOutputStream buf = new ByteArrayOutputStream();
+               PacketLineOut pckOut = new PacketLineOut(buf);
+               PacketLineOutRefAdvertiser adv = new PacketLineOutRefAdvertiser(pckOut);
+
+               // Advertisement of capability and id both happen in order of call,
+               // which may not match Git standards. Callers are responsible for
+               // making calls in the correct ordering. Here in this test we do them
+               // in a "wrong" order to assert the method just writes to the network.
+
+               adv.advertiseCapability("test-1");
+               adv.advertiseCapability("sideband");
+               adv.advertiseId(id(1), "refs/heads/master");
+               adv.advertiseId(id(4), "refs/" + padStart("F", 987));
+               adv.advertiseId(id(2), "refs/heads/next");
+               adv.advertiseId(id(3), "refs/Iñtërnâtiônàlizætiøn☃💩");
+               adv.end();
+               assertFalse(adv.isEmpty());
+
+               PacketLineIn pckIn = new PacketLineIn(
+                               new ByteArrayInputStream(buf.toByteArray()));
+               String s = pckIn.readStringRaw();
+               assertEquals(id(1).name() + " refs/heads/master\0 test-1 sideband\n",
+                               s);
+
+               s = pckIn.readStringRaw();
+               assertEquals(id(4).name() + " refs/" + padStart("F", 987) + '\n', s);
+
+               s = pckIn.readStringRaw();
+               assertEquals(id(2).name() + " refs/heads/next\n", s);
+
+               s = pckIn.readStringRaw();
+               assertEquals(id(3).name() + " refs/Iñtërnâtiônàlizætiøn☃💩\n", s);
+
+               s = pckIn.readStringRaw();
+               assertSame(PacketLineIn.END, s);
+       }
+
+       private static ObjectId id(int i) {
+               try (ObjectInserter.Formatter f = new ObjectInserter.Formatter()) {
+                       byte[] tmp = new byte[4];
+                       NB.encodeInt32(tmp, 0, i);
+                       return f.idFor(Constants.OBJ_BLOB, tmp);
+               }
+       }
+
+       private static String padStart(String s, int len) {
+               StringBuilder b = new StringBuilder(len);
+               for (int i = s.length(); i < len; i++) {
+                       b.append((char) ('a' + (i % 26)));
+               }
+               return b.append(s).toString();
+       }
+}
index bb83dce48dfac97b27f3af041e0bdca8833a5ad7..2d4e9c717f699432e9721b245b8d1518bb01fa56 100644 (file)
@@ -113,10 +113,28 @@ public class PacketLineOut {
         *             the packet could not be written, the stream is corrupted as
         *             the packet may have been only partially written.
         */
-       public void writePacket(final byte[] packet) throws IOException {
-               formatLength(packet.length + 4);
+       public void writePacket(byte[] packet) throws IOException {
+               writePacket(packet, 0, packet.length);
+       }
+
+       /**
+        * Write a binary packet to the stream.
+        *
+        * @param buf
+        *            the packet to write
+        * @param pos
+        *            first index within {@code buf}.
+        * @param len
+        *            number of bytes to write.
+        * @throws IOException
+        *             the packet could not be written, the stream is corrupted as
+        *             the packet may have been only partially written.
+        * @since 4.5
+        */
+       public void writePacket(byte[] buf, int pos, int len) throws IOException {
+               formatLength(len + 4);
                out.write(lenbuffer, 0, 4);
-               out.write(packet);
+               out.write(buf, pos, len);
        }
 
        /**
index f72a4b2b30bacac428733594c52f3bc68711d264..0cd720c29d2fc159194dda44bb7da80a8eeaa46b 100644 (file)
 
 package org.eclipse.jgit.transport;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jgit.lib.Constants.OBJECT_ID_STRING_LENGTH;
 import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SYMREF;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CoderResult;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -64,8 +71,15 @@ import org.eclipse.jgit.util.RefMap;
 public abstract class RefAdvertiser {
        /** Advertiser which frames lines in a {@link PacketLineOut} format. */
        public static class PacketLineOutRefAdvertiser extends RefAdvertiser {
+               private final CharsetEncoder utf8 = UTF_8.newEncoder();
                private final PacketLineOut pckOut;
 
+               private byte[] binArr = new byte[256];
+               private ByteBuffer binBuf = ByteBuffer.wrap(binArr);
+
+               private char[] chArr = new char[256];
+               private CharBuffer chBuf = CharBuffer.wrap(chArr);
+
                /**
                 * Create a new advertiser for the supplied stream.
                 *
@@ -76,6 +90,64 @@ public abstract class RefAdvertiser {
                        pckOut = out;
                }
 
+               @Override
+               public void advertiseId(AnyObjectId id, String refName)
+                               throws IOException {
+                       id.copyTo(binArr, 0);
+                       binArr[OBJECT_ID_STRING_LENGTH] = ' ';
+                       binBuf.position(OBJECT_ID_STRING_LENGTH + 1);
+                       append(refName);
+                       if (first) {
+                               first = false;
+                               if (!capablities.isEmpty()) {
+                                       append('\0');
+                                       for (String cap : capablities) {
+                                               append(' ');
+                                               append(cap);
+                                       }
+                               }
+                       }
+                       append('\n');
+                       pckOut.writePacket(binArr, 0, binBuf.position());
+               }
+
+               private void append(String str) throws CharacterCodingException {
+                       int n = str.length();
+                       if (n > chArr.length) {
+                               chArr = new char[n + 256];
+                               chBuf = CharBuffer.wrap(chArr);
+                       }
+                       str.getChars(0, n, chArr, 0);
+                       chBuf.position(0).limit(n);
+                       utf8.reset();
+                       for (;;) {
+                               CoderResult cr = utf8.encode(chBuf, binBuf, true);
+                               if (cr.isOverflow()) {
+                                       grow();
+                               } else if (cr.isUnderflow()) {
+                                       break;
+                               } else {
+                                       cr.throwException();
+                               }
+                       }
+               }
+
+               private void append(int b) {
+                       if (!binBuf.hasRemaining()) {
+                               grow();
+                       }
+                       binBuf.put((byte) b);
+               }
+
+               private void grow() {
+                       int cnt = binBuf.position();
+                       byte[] tmp = new byte[binArr.length << 1];
+                       System.arraycopy(binArr, 0, tmp, 0, cnt);
+                       binArr = tmp;
+                       binBuf = ByteBuffer.wrap(binArr);
+                       binBuf.position(cnt);
+               }
+
                @Override
                protected void writeOne(final CharSequence line) throws IOException {
                        pckOut.writeString(line.toString());
@@ -91,7 +163,7 @@ public abstract class RefAdvertiser {
 
        private final char[] tmpId = new char[Constants.OBJECT_ID_STRING_LENGTH];
 
-       private final Set<String> capablities = new LinkedHashSet<String>();
+       final Set<String> capablities = new LinkedHashSet<String>();
 
        private final Set<ObjectId> sent = new HashSet<ObjectId>();
 
@@ -99,7 +171,7 @@ public abstract class RefAdvertiser {
 
        private boolean derefTags;
 
-       private boolean first = true;
+       boolean first = true;
 
        /**
         * Initialize this advertiser with a repository for peeling tags.