Optimize RefAdvertiser performance by avoiding sorting

Don't copy and sort the set of references if they are passed through
in a RefMap or a SortedMap using the key's natural sort ordering.
Either map is already in the order we want to present the items
to the client in, so copying and sorting is a waste of local CPU
and memory.

Change-Id: I49ada7c1220e0fc2a163b9752c2b77525d9c82c1
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java
index 3b615198a0d5bf737e006b8ea3073c15119b0449..b766196fdcc5cc5e4a32e4076d4a741a64be61e2 100644 (file)
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java
@@ -99,7 +99,7 @@ class InfoRefsServlet extends HttpServlet {
 
                Map<String, Ref> refs = db.getAllRefs();
                refs.remove(Constants.HEAD);
-               adv.send(refs.values());
+               adv.send(refs);
                return out.toString().getBytes(Constants.CHARACTER_ENCODING);
        }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
index f8be827dfdb019846ed8279f08931891fec121dd..15bdf9618f982c3bc6805b9b0fcd67309c19f3b6 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -591,7 +591,7 @@ public class ReceivePack {
                        adv.advertiseCapability(CAPABILITY_OFS_DELTA);
                refs = db.getAllRefs();
                final Ref head = refs.remove(Constants.HEAD);
-               adv.send(refs.values());
+               adv.send(refs);
                if (!head.isSymbolic())
                        adv.advertiseHave(head.getObjectId());
                adv.includeAdditionalHaves();

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 77f30140c6b14e143276cca3416f2d91d04d94ce..694a2e0f169944e0d5c4638b6243c3e3389eb6ad 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefAdvertiser.java
@@ -44,9 +44,10 @@
 package org.eclipse.jgit.transport;
 
 import java.io.IOException;
-import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
 
 import org.eclipse.jgit.lib.AlternateRepositoryDatabase;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -59,6 +60,7 @@ import org.eclipse.jgit.revwalk.RevFlag;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevTag;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.RefMap;
 
 /** Support for the start of {@link UploadPack} and {@link ReceivePack}. */
 public abstract class RefAdvertiser {
@@ -122,7 +124,7 @@ public abstract class RefAdvertiser {
         * <p>
         * This method must be invoked prior to any of the following:
         * <ul>
-        * <li>{@link #send(Collection)}
+        * <li>{@link #send(Map)}
         * <li>{@link #advertiseHave(AnyObjectId)}
         * <li>{@link #includeAdditionalHaves()}
         * </ul>
@@ -140,7 +142,7 @@ public abstract class RefAdvertiser {
         * <p>
         * This method must be invoked prior to any of the following:
         * <ul>
-        * <li>{@link #send(Collection)}
+        * <li>{@link #send(Map)}
         * <li>{@link #advertiseHave(AnyObjectId)}
         * <li>{@link #includeAdditionalHaves()}
         * </ul>
@@ -160,14 +162,14 @@ public abstract class RefAdvertiser {
         *
         * @param refs
         *            zero or more refs to format for the client. The collection is
-        *            copied and sorted before display and therefore may appear in
-        *            any order.
+        *            sorted before display if necessary, and therefore may appear
+        *            in any order.
         * @throws IOException
         *             the underlying output stream failed to write out an
         *             advertisement record.
         */
-       public void send(final Collection<Ref> refs) throws IOException {
-               for (final Ref r : RefComparator.sort(refs)) {
+       public void send(final Map<String, Ref> refs) throws IOException {
+               for (final Ref r : getSortedRefs(refs)) {
                        final RevObject obj = parseAnyOrNull(r.getObjectId());
                        if (obj != null) {
                                advertiseAny(obj, r.getName());
@@ -177,6 +179,13 @@ public abstract class RefAdvertiser {
                }
        }
 
+       private Iterable<Ref> getSortedRefs(Map<String, Ref> all) {
+               if (all instanceof RefMap
+                               || (all instanceof SortedMap && ((SortedMap) all).comparator() == null))
+                       return all.values();
+               return RefComparator.sort(all.values());
+       }
+
        /**
         * Advertise one object is available using the magic {@code .have}.
         * <p>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 6b81bc4925646c9d1fc8cae2f84ea14982686ad8..57bb2adbf86cc7cad8e696989719f222eff44c8d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2008-2010, Google Inc.
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -330,7 +330,7 @@ public class UploadPack {
                adv.advertiseCapability(OPTION_NO_PROGRESS);
                adv.setDerefTags(true);
                refs = db.getAllRefs();
-               adv.send(refs.values());
+               adv.send(refs);
                adv.end();
        }