Merge branch 'master' into develop/2.8

Author: supersaiyansubtlety

CHANGELOG.md

@@ -486,3 +486,12 @@ Blood, sweat, refactors, enhancements, tears, unit tests, bugfixes, blood, and m
 VINEFLOWERRRRRRRRRR
 
 - downgrade vineflower to 1.11.1 to fix issues with decompilation ([#341](https://github.com/QuiltMC/enigma/pull/341))
+
+# 2.7.2
+
+version `2.7.2`, or "your beloved enigma team proves once again that they are professionals at introducing performance issues".
+
+- fix memory overuse issues introduced with the library index changes ([#345](https://github.com/QuiltMC/enigma/pull/345))
+  - indexes are now better combined internally to save memory
+- define the iteration order for `InheritanceIndex#getAncestors(ClassEntry)` as breadth-first ([#347](https://github.com/QuiltMC/enigma/pull/347))
+  - add `InheritanceIndex#streamAncestors(ClassEntry)`

build.gradle

@@ -33,7 +33,7 @@ subprojects {
 
 	group = 'org.quiltmc'
 	// When bumping version, remember to update the int constants in Enigma!
-	version = '2.7.1'
+	version = '2.7.2'
 
 	var ENV = System.getenv()
 	version = version + (ENV.GITHUB_ACTIONS ? (ENV.SNAPSHOTS_URL ? "-SNAPSHOT" : "") : "+local")

enigma/build.gradle

@@ -376,7 +376,8 @@ final testRecommendedImplPluginAgainstBumpedEnigma = tasks
 	}
 }
 
-test.dependsOn(obfuscateTestInputs, testPreHandlingPluginAgainstCurrentEnigma, testRecommendedImplPluginAgainstBumpedEnigma)
+test.dependsOn(obfuscateTestInputs)
+check.dependsOn(testPreHandlingPluginAgainstCurrentEnigma, testRecommendedImplPluginAgainstBumpedEnigma)
 
 publishing {
 	publications {

enigma/src/main/java/org/quiltmc/enigma/api/Enigma.java

@@ -82,7 +82,7 @@ public class Enigma {
 	 * {@link EnigmaPlugin#supportsEnigmaVersion(Version)} implementations,
 	 * allowing plugins to 'remember' the version of Enigma they were built with.
 	 */
-	public static final int PATCH_VERSION = 1;
+	public static final int PATCH_VERSION = 2;
 
 	private static final Version CURRENT_VERSION = new Version(MAJOR_VERSION, MINOR_VERSION, PATCH_VERSION);
 

enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/InheritanceIndex.java

@@ -2,14 +2,18 @@
 
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
 import org.quiltmc.enigma.api.translation.representation.entry.ClassDefEntry;
 import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
+import org.quiltmc.enigma.util.Utils;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.Set;
+import java.util.stream.Stream;
+
+import static java.util.stream.Collectors.toCollection;
 
 public class InheritanceIndex implements JarIndexer {
 	private final EntryIndex entryIndex;
@@ -63,21 +67,52 @@ public Collection<ClassEntry> getDescendants(ClassEntry classEntry) {
 		return descendants;
 	}
 
+	/**
+	 * Prefer {@link #streamAncestors(ClassEntry)} if:
+	 * <ul>
+	 *     <li> performing a search that may terminate before examining all ancestors
+	 *     <li> the creation of a {@link Set} is unnecessary
+	 *     <li> duplicate occurrences of interfaces implemented by multiple ancestors are required
+	 * </ul>
+	 *
+	 * @return a {@link Set} containing the passed {@code classEntry}'s ancestors
+	 *
+	 * @implSpec The returned set has breadth-first iteration order.<br>
+	 *           Only the first (shallowest) occurrence of an interface implemented by multiple ancestors is included.<br>
+	 *           Only the first (shallowest) occurrence of {@code java.lang.Object} is included.
+	 *
+	 * @implNote No guarantees are made about the order within a generation of ancestors.
+	 *
+	 * @see #streamAncestors(ClassEntry)
+	 */
 	public Set<ClassEntry> getAncestors(ClassEntry classEntry) {
-		Set<ClassEntry> ancestors = Sets.newHashSet();
-
-		LinkedList<ClassEntry> ancestorQueue = new LinkedList<>();
-		ancestorQueue.push(classEntry);
-
-		while (!ancestorQueue.isEmpty()) {
-			ClassEntry ancestor = ancestorQueue.pop();
-			Collection<ClassEntry> parents = this.getParents(ancestor);
+		return this.streamAncestors(classEntry).collect(toCollection(LinkedHashSet::new));
+	}
 
-			parents.forEach(ancestorQueue::push);
-			ancestors.addAll(parents);
-		}
+	/**
+	 * @return a {@link Stream} of the passed {@code classEntry}'s ancestors in breadth-first order
+	 *
+	 * @implSpec Interfaces implemented by multiple ancestors will occur multiple times in the stream
+	 *           (see {@link Stream#distinct()} and {@link #getAncestors(ClassEntry)}).<br>
+	 *           {@code java.lang.Object} will occur once for each interface.
+	 *
+	 * @implNote No guarantees are made about the order within a generation of ancestors.
+	 *
+	 * @see #getAncestors(ClassEntry)
+	 */
+	public Stream<ClassEntry> streamAncestors(ClassEntry classEntry) {
+		return this.streamAncestorsImpl(this.getParents(classEntry));
+	}
 
-		return ancestors;
+	private Stream<ClassEntry> streamAncestorsImpl(Collection<ClassEntry> generation) {
+		return generation.isEmpty() ? Stream.empty() : Utils.lazyConcat(
+			generation::stream,
+			() -> this.streamAncestorsImpl(generation.stream()
+				.map(this::getParents)
+				.flatMap(Collection::stream)
+				.toList()
+			)
+		);
 	}
 
 	public Relation computeClassRelation(ClassEntry classEntry, ClassEntry potentialAncestor) {

enigma/src/main/java/org/quiltmc/enigma/util/Utils.java

@@ -21,6 +21,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.function.Supplier;
+import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 
@@ -196,4 +197,9 @@ public static float clamp(double value, float min, float max) {
 	public static double clamp(double value, double min, double max) {
 		return Math.min(max, Math.max(value, min));
 	}
+
+	@SafeVarargs
+	public static <T> Stream<T> lazyConcat(Supplier<Stream<? extends T>>... streamers) {
+		return Stream.of(streamers).flatMap(Supplier::get);
+	}
 }

enigma/src/test/java/org/quiltmc/enigma/TestJarIndexInheritanceTree.java

@@ -23,6 +23,7 @@
 
 import java.nio.file.Path;
 import java.util.Collection;
+import java.util.List;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
@@ -35,9 +36,9 @@ public class TestJarIndexInheritanceTree {
 
 	private static final ClassEntry OBJECT_CLASS = TestEntryFactory.newClass("java/lang/Object");
 	private static final ClassEntry BASE_CLASS = TestEntryFactory.newClass("a");
-	private static final ClassEntry SUB_CLASS_A = TestEntryFactory.newClass("b");
-	private static final ClassEntry SUB_CLASS_AA = TestEntryFactory.newClass("d");
-	private static final ClassEntry SUB_CLASS_B = TestEntryFactory.newClass("c");
+	private static final ClassEntry SUB_CLASS_A = TestEntryFactory.newClass("c");
+	private static final ClassEntry SUB_CLASS_AA = TestEntryFactory.newClass("e");
+	private static final ClassEntry SUB_CLASS_B = TestEntryFactory.newClass("d");
 	private static final FieldEntry NAME_FIELD = TestEntryFactory.newField(BASE_CLASS, "a", "Ljava/lang/String;");
 	private static final FieldEntry NUM_THINGS_FIELD = TestEntryFactory.newField(SUB_CLASS_B, "a", "I");
 
@@ -52,7 +53,12 @@ public TestJarIndexInheritanceTree() throws Exception {
 	@Test
 	public void obfEntries() {
 		assertThat(this.index.getIndex(EntryIndex.class).getClasses(), Matchers.containsInAnyOrder(
-				TestEntryFactory.newClass("org/quiltmc/enigma/input/Keep"), BASE_CLASS, SUB_CLASS_A, SUB_CLASS_AA, SUB_CLASS_B
+				TestEntryFactory.newClass("org/quiltmc/enigma/input/Keep"),
+				BASE_CLASS, SUB_CLASS_A, SUB_CLASS_AA, SUB_CLASS_B,
+				InterfaceTree.OUTER,
+				InterfaceTree.ROOT,
+				InterfaceTree.BRANCH_1_2, InterfaceTree.BRANCH_3_4,
+				InterfaceTree.LEAF_1, InterfaceTree.LEAF_2, InterfaceTree.LEAF_3, InterfaceTree.LEAF_4
 		));
 	}
 
@@ -216,4 +222,65 @@ public void containsEntries() {
 		assertThat(entryIndex.hasMethod(TestEntryFactory.newMethod(SUB_CLASS_AA, "b", "()V")), is(false));
 		assertThat(entryIndex.hasMethod(TestEntryFactory.newMethod(SUB_CLASS_B, "b", "()V")), is(true));
 	}
+
+	@Test
+	void streamAncestorsOrder() {
+		final List<ClassEntry> ancestors = this.index.getIndex(InheritanceIndex.class)
+				.streamAncestors(InterfaceTree.ROOT)
+				.toList();
+
+		assertThat(ancestors.size(), is(13));
+
+		// First generation: Object appears once for ROOT
+		assertThat(ancestors.subList(0, 3), Matchers.containsInAnyOrder(
+				InterfaceTree.BRANCH_1_2, InterfaceTree.BRANCH_3_4, OBJECT_CLASS
+		));
+
+		// Second generation: Object appears once for each of the 2 branches
+		assertThat(ancestors.subList(3, 9), Matchers.containsInAnyOrder(
+				InterfaceTree.LEAF_1, InterfaceTree.LEAF_2, InterfaceTree.LEAF_3, InterfaceTree.LEAF_4,
+				OBJECT_CLASS, OBJECT_CLASS
+		));
+
+		// Third generation: Object appears once for each of the 4 leaves
+		assertThat(ancestors.subList(9, 13), Matchers.containsInAnyOrder(
+				OBJECT_CLASS, OBJECT_CLASS, OBJECT_CLASS, OBJECT_CLASS
+		));
+	}
+
+	@Test
+	void getAncestorsOrder() {
+		final List<ClassEntry> ancestors = this.index.getIndex(InheritanceIndex.class)
+				.getAncestors(InterfaceTree.ROOT)
+				.stream()
+				.toList();
+
+		assertThat(ancestors.size(), is(7));
+
+		// Only the first appearance of Object (from the root) is included
+		assertThat(ancestors.subList(0, 3), Matchers.containsInAnyOrder(
+				InterfaceTree.BRANCH_1_2, InterfaceTree.BRANCH_3_4, OBJECT_CLASS
+		));
+
+		assertThat(ancestors.subList(3, 7), Matchers.containsInAnyOrder(
+				InterfaceTree.LEAF_1, InterfaceTree.LEAF_2, InterfaceTree.LEAF_3, InterfaceTree.LEAF_4
+		));
+	}
+
+	private interface InterfaceTree {
+		String OUTER_NAME = "b";
+		String OUTER_PREFIX = OUTER_NAME + "$";
+
+		ClassEntry OUTER = TestEntryFactory.newClass(OUTER_NAME);
+
+		ClassEntry ROOT = TestEntryFactory.newClass(OUTER_PREFIX + "g");
+
+		ClassEntry BRANCH_1_2 = TestEntryFactory.newClass(OUTER_PREFIX + "a");
+		ClassEntry BRANCH_3_4 = TestEntryFactory.newClass(OUTER_PREFIX + "b");
+
+		ClassEntry LEAF_1 = TestEntryFactory.newClass(OUTER_PREFIX + "c");
+		ClassEntry LEAF_2 = TestEntryFactory.newClass(OUTER_PREFIX + "d");
+		ClassEntry LEAF_3 = TestEntryFactory.newClass(OUTER_PREFIX + "e");
+		ClassEntry LEAF_4 = TestEntryFactory.newClass(OUTER_PREFIX + "f");
+	}
 }

enigma/src/test/java/org/quiltmc/enigma/input/inheritance_tree/InterfaceTree.java

@@ -0,0 +1,17 @@
+package org.quiltmc.enigma.input.inheritance_tree;
+
+public class InterfaceTree {
+	interface Root extends Branch12, Branch34 { }
+
+	interface Branch12 extends Leaf1, Leaf2 { }
+
+	interface Branch34 extends Leaf3, Leaf4 { }
+
+	interface Leaf1 { }
+
+	interface Leaf2 { }
+
+	interface Leaf3 { }
+
+	interface Leaf4 { }
+}