Skip to content
Snippets Groups Projects
Commit 36f05a9c authored by Shawn Pearce's avatar Shawn Pearce
Browse files

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: default avatarShawn O. Pearce <spearce@spearce.org>
parent 57f6f6a6
No related branches found
No related tags found
No related merge requests found
...@@ -99,7 +99,7 @@ protected void end() { ...@@ -99,7 +99,7 @@ protected void end() {
Map<String, Ref> refs = db.getAllRefs(); Map<String, Ref> refs = db.getAllRefs();
refs.remove(Constants.HEAD); refs.remove(Constants.HEAD);
adv.send(refs.values()); adv.send(refs);
return out.toString().getBytes(Constants.CHARACTER_ENCODING); return out.toString().getBytes(Constants.CHARACTER_ENCODING);
} }
} }
...@@ -591,7 +591,7 @@ public void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException { ...@@ -591,7 +591,7 @@ public void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException {
adv.advertiseCapability(CAPABILITY_OFS_DELTA); adv.advertiseCapability(CAPABILITY_OFS_DELTA);
refs = db.getAllRefs(); refs = db.getAllRefs();
final Ref head = refs.remove(Constants.HEAD); final Ref head = refs.remove(Constants.HEAD);
adv.send(refs.values()); adv.send(refs);
if (!head.isSymbolic()) if (!head.isSymbolic())
adv.advertiseHave(head.getObjectId()); adv.advertiseHave(head.getObjectId());
adv.includeAdditionalHaves(); adv.includeAdditionalHaves();
......
...@@ -44,9 +44,10 @@ ...@@ -44,9 +44,10 @@
package org.eclipse.jgit.transport; package org.eclipse.jgit.transport;
import java.io.IOException; import java.io.IOException;
import java.util.Collection;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.SortedMap;
import org.eclipse.jgit.lib.AlternateRepositoryDatabase; import org.eclipse.jgit.lib.AlternateRepositoryDatabase;
import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.AnyObjectId;
...@@ -59,6 +60,7 @@ ...@@ -59,6 +60,7 @@
import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevTag;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.RefMap;
/** Support for the start of {@link UploadPack} and {@link ReceivePack}. */ /** Support for the start of {@link UploadPack} and {@link ReceivePack}. */
public abstract class RefAdvertiser { public abstract class RefAdvertiser {
...@@ -122,7 +124,7 @@ public void init(final RevWalk protoWalk, final RevFlag advertisedFlag) { ...@@ -122,7 +124,7 @@ public void init(final RevWalk protoWalk, final RevFlag advertisedFlag) {
* <p> * <p>
* This method must be invoked prior to any of the following: * This method must be invoked prior to any of the following:
* <ul> * <ul>
* <li>{@link #send(Collection)} * <li>{@link #send(Map)}
* <li>{@link #advertiseHave(AnyObjectId)} * <li>{@link #advertiseHave(AnyObjectId)}
* <li>{@link #includeAdditionalHaves()} * <li>{@link #includeAdditionalHaves()}
* </ul> * </ul>
...@@ -140,7 +142,7 @@ public void setDerefTags(final boolean deref) { ...@@ -140,7 +142,7 @@ public void setDerefTags(final boolean deref) {
* <p> * <p>
* This method must be invoked prior to any of the following: * This method must be invoked prior to any of the following:
* <ul> * <ul>
* <li>{@link #send(Collection)} * <li>{@link #send(Map)}
* <li>{@link #advertiseHave(AnyObjectId)} * <li>{@link #advertiseHave(AnyObjectId)}
* <li>{@link #includeAdditionalHaves()} * <li>{@link #includeAdditionalHaves()}
* </ul> * </ul>
...@@ -160,14 +162,14 @@ public void advertiseCapability(String name) { ...@@ -160,14 +162,14 @@ public void advertiseCapability(String name) {
* *
* @param refs * @param refs
* zero or more refs to format for the client. The collection is * zero or more refs to format for the client. The collection is
* copied and sorted before display and therefore may appear in * sorted before display if necessary, and therefore may appear
* any order. * in any order.
* @throws IOException * @throws IOException
* the underlying output stream failed to write out an * the underlying output stream failed to write out an
* advertisement record. * advertisement record.
*/ */
public void send(final Collection<Ref> refs) throws IOException { public void send(final Map<String, Ref> refs) throws IOException {
for (final Ref r : RefComparator.sort(refs)) { for (final Ref r : getSortedRefs(refs)) {
final RevObject obj = parseAnyOrNull(r.getObjectId()); final RevObject obj = parseAnyOrNull(r.getObjectId());
if (obj != null) { if (obj != null) {
advertiseAny(obj, r.getName()); advertiseAny(obj, r.getName());
...@@ -177,6 +179,13 @@ public void send(final Collection<Ref> refs) throws IOException { ...@@ -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}. * Advertise one object is available using the magic {@code .have}.
* <p> * <p>
......
/* /*
* Copyright (C) 2008-2009, Google Inc. * Copyright (C) 2008-2010, Google Inc.
* and other copyright owners as documented in the project's IP log. * and other copyright owners as documented in the project's IP log.
* *
* This program and the accompanying materials are made available * This program and the accompanying materials are made available
...@@ -330,7 +330,7 @@ public void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException { ...@@ -330,7 +330,7 @@ public void sendAdvertisedRefs(final RefAdvertiser adv) throws IOException {
adv.advertiseCapability(OPTION_NO_PROGRESS); adv.advertiseCapability(OPTION_NO_PROGRESS);
adv.setDerefTags(true); adv.setDerefTags(true);
refs = db.getAllRefs(); refs = db.getAllRefs();
adv.send(refs.values()); adv.send(refs);
adv.end(); adv.end();
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment