From 2ec3accb3bcc0bd45dfb1333511c72489ab835cb Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 18 Jun 2016 21:40:35 -0700 Subject: [PATCH] Optimize RefAdvertiser for wire protocol 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 --- .../jgit/transport/RefAdvertiserTest.java | 116 ++++++++++++++++++ .../eclipse/jgit/transport/PacketLineOut.java | 24 +++- .../eclipse/jgit/transport/RefAdvertiser.java | 76 +++++++++++- 3 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefAdvertiserTest.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 index 000000000..ce69adf9c --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefAdvertiserTest.java @@ -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(); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java index bb83dce48..2d4e9c717 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineOut.java @@ -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); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java index f72a4b2b3..0cd720c29 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java @@ -43,9 +43,16 @@ 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 capablities = new LinkedHashSet(); + final Set capablities = new LinkedHashSet(); private final Set sent = new HashSet(); @@ -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.