From 47e37b0fa4c91aa0fe3edfc298e93ddeed4c5473 Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Tue, 19 Mar 2019 20:42:10 -0700 Subject: [PATCH] Use RefMap instead of HashMap HashMap has a memory overhead for refs. Use RefMap. Change-Id: I3fb4616135dacf687cc3bc2b473effc66ccef5e6 Signed-off-by: Masaya Suzuki --- .../org/eclipse/jgit/util/RefListTest.java | 35 ++++++++++ .../eclipse/jgit/transport/UploadPack.java | 11 ++-- .../src/org/eclipse/jgit/util/RefList.java | 64 +++++++++++++++++++ .../src/org/eclipse/jgit/util/RefMap.java | 18 ++++++ 4 files changed, 122 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RefListTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RefListTest.java index d124d7365..9981bd651 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RefListTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RefListTest.java @@ -127,6 +127,41 @@ public class RefListTest { assertSame(REF_B, list.get(1)); } + @Test + public void testBuilder_AddThenDedupe() { + RefList.Builder builder = new RefList.Builder<>(1); + builder.add(REF_B); + builder.add(REF_A); + builder.add(REF_A); + builder.add(REF_B); + builder.add(REF_c); + + builder.sort(); + builder.dedupe((a, b) -> b); + RefList list = builder.toRefList(); + + assertEquals(3, list.size()); + assertSame(REF_A, list.get(0)); + assertSame(REF_B, list.get(1)); + assertSame(REF_c, list.get(2)); + } + + @Test + public void testBuilder_AddThenDedupe_Border() { + RefList.Builder builder = new RefList.Builder<>(1); + builder.sort(); + builder.dedupe((a, b) -> b); + RefList list = builder.toRefList(); + assertTrue(list.isEmpty()); + + builder = new RefList.Builder<>(1); + builder.add(REF_A); + builder.sort(); + builder.dedupe((a, b) -> b); + list = builder.toRefList(); + assertEquals(1, list.size()); + } + @Test public void testBuilder_AddAll() { RefList.Builder builder = new RefList.Builder<>(1); 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 fe3e8141f..dce06d305 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -44,8 +44,7 @@ package org.eclipse.jgit.transport; import static java.util.Collections.unmodifiableMap; -import static java.util.function.Function.identity; -import static java.util.stream.Collectors.toMap; +import static org.eclipse.jgit.util.RefMap.toRefMap; import static org.eclipse.jgit.lib.Constants.R_TAGS; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT; import static org.eclipse.jgit.transport.GitProtocolConstants.COMMAND_FETCH; @@ -817,7 +816,7 @@ public class UploadPack { // Fall back to all refs. setAdvertisedRefs( db.getRefDatabase().getRefs().stream() - .collect(toMap(Ref::getName, identity()))); + .collect(toRefMap((a, b) -> b))); } return refs; } @@ -836,7 +835,7 @@ public class UploadPack { String[] prefixes = refPrefixes.toArray(new String[0]); Map rs = db.getRefDatabase().getRefsByPrefix(prefixes).stream() - .collect(toMap(Ref::getName, identity(), (a, b) -> b)); + .collect(toRefMap((a, b) -> b)); if (refFilter != RefFilter.DEFAULT) { return refFilter.filter(rs); } @@ -848,7 +847,7 @@ public class UploadPack { return refs.values().stream() .filter(ref -> refPrefixes.stream() .anyMatch(ref.getName()::startsWith)) - .collect(toMap(Ref::getName, identity())); + .collect(toRefMap((a, b) -> b)); } /** @@ -871,7 +870,7 @@ public class UploadPack { names.stream() .map(refs::get) .filter(Objects::nonNull) - .collect(toMap(Ref::getName, identity(), (a, b) -> b))); + .collect(toRefMap((a, b) -> b))); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java index 639c35362..60dead51b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefList.java @@ -48,7 +48,10 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; +import java.util.function.BinaryOperator; +import java.util.stream.Collector; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefComparator; @@ -332,6 +335,32 @@ public class RefList implements Iterable { return r.toString(); } + /** + * Create a {@link Collector} for {@link Ref}. + * + * @param mergeFunction + * if specified the result will be sorted and deduped. + * @return {@link Collector} for {@link Ref} + * @since 5.4 + */ + public static Collector> toRefList( + @Nullable BinaryOperator mergeFunction) { + return Collector.of( + () -> new Builder<>(), + Builder::add, (b1, b2) -> { + Builder b = new Builder<>(); + b.addAll(b1); + b.addAll(b2); + return b; + }, (b) -> { + if (mergeFunction != null) { + b.sort(); + b.dedupe(mergeFunction); + } + return b.toRefList(); + }); + } + /** * Builder to facilitate fast construction of an immutable RefList. * @@ -404,6 +433,16 @@ public class RefList implements Iterable { list[size++] = ref; } + /** + * Add all items from another builder. + * + * @param other + * @since 5.4 + */ + public void addAll(Builder other) { + addAll(other.list, 0, other.size); + } + /** * Add all items from a source array. *

@@ -444,6 +483,31 @@ public class RefList implements Iterable { Arrays.sort(list, 0, size, RefComparator.INSTANCE); } + /** + * Dedupe the refs in place. Must be called after {@link #sort}. + * + * @param mergeFunction + */ + @SuppressWarnings("unchecked") + void dedupe(BinaryOperator mergeFunction) { + if (size == 0) { + return; + } + int lastElement = 0; + for (int i = 1; i < size; i++) { + if (RefComparator.INSTANCE.compare(list[lastElement], + list[i]) == 0) { + list[lastElement] = mergeFunction + .apply((T) list[lastElement], (T) list[i]); + } else { + list[lastElement + 1] = list[i]; + lastElement++; + } + } + size = lastElement + 1; + Arrays.fill(list, size, list.length, null); + } + /** @return an unmodifiable list using this collection's backing array. */ public RefList toRefList() { return new RefList<>(list, size); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java index a3f9730f1..d7a4c2535 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java @@ -49,6 +49,9 @@ import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; +import java.util.function.BinaryOperator; +import java.util.stream.Collector; +import java.util.stream.Collectors; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.ObjectId; @@ -285,6 +288,21 @@ public class RefMap extends AbstractMap { return r.toString(); } + /** + * Create a {@link Collector} for {@link Ref}. + * + * @param mergeFunction + * @return {@link Collector} for {@link Ref} + * @since 5.4 + */ + public static Collector toRefMap( + BinaryOperator mergeFunction) { + return Collectors.collectingAndThen(RefList.toRefList(mergeFunction), + (refs) -> new RefMap("", //$NON-NLS-1$ + refs, RefList.emptyList(), + RefList.emptyList())); + } + private String toRefName(String name) { if (0 < prefix.length()) name = prefix + name;