From 36f05a9c27e6961b10df0b65014ffc869f4f8686 Mon Sep 17 00:00:00 2001
From: "Shawn O. Pearce" <spearce@spearce.org>
Date: Fri, 22 Jan 2010 18:42:12 -0800
Subject: [PATCH] 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>
---
.../jgit/http/server/InfoRefsServlet.java | 2 +-
.../eclipse/jgit/transport/ReceivePack.java | 2 +-
.../eclipse/jgit/transport/RefAdvertiser.java | 23 +++++++++++++------
.../eclipse/jgit/transport/UploadPack.java | 4 ++--
4 files changed, 20 insertions(+), 11 deletions(-)
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 3b615198a..b766196fd 100644
--- 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 @@ protected void end() {
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 f8be827df..15bdf9618 100644
--- 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 void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException {
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 77f30140c..694a2e0f1 100644
--- 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.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 void init(final RevWalk protoWalk, final RevFlag advertisedFlag) {
* <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 void setDerefTags(final boolean deref) {
* <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 void advertiseCapability(String name) {
*
* @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 void send(final Collection<Ref> refs) throws IOException {
}
}
+ 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 6b81bc492..57bb2adbf 100644
--- 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 void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException {
adv.advertiseCapability(OPTION_NO_PROGRESS);
adv.setDerefTags(true);
refs = db.getAllRefs();
- adv.send(refs.values());
+ adv.send(refs);
adv.end();
}
--
GitLab