Version 1.0.3, complete mappings & performance improvments

DenWav · DenWav · commit 990ed7135bda · 2023-07-30T14:26:51.000-05:00
Read jars into Hypo in a single pass and re-use it for subsequent pages.
Also, fix jars using Hypo's executor service in parallel.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -22,6 +22,8 @@ dependencies {
     implementation(platform(libs.hypo.platform))
     implementation(libs.bundles.hypo)
 
+    implementation(libs.lorenz)
+
     implementation(libs.feather.core)
     implementation(libs.feather.gson)
 
diff --git a/gradle.properties b/gradle.properties
@@ -1,2 +1,2 @@
 group = io.papermc.codebook
-version = 1.0.3-SNAPSHOT
+version = 1.0.3
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -41,12 +41,15 @@ asm-tree = { module = "org.ow2.asm:asm-tree", version.ref = "asm" }
 unpick-format = { module = "net.fabricmc.unpick:unpick-format-utils", version.ref = "unpick" }
 unpick-cli = { module = "net.fabricmc.unpick:unpick-cli", version.ref = "unpick" }
 
-hypo-platform = "dev.denwav.hypo:hypo-platform:2.0.0"
+hypo-platform = "dev.denwav.hypo:hypo-platform:2.1.0"
 hypo-model = { module = "dev.denwav.hypo:hypo-model" }
 hypo-core = { module = "dev.denwav.hypo:hypo-core" }
 hypo-asm = { module = "dev.denwav.hypo:hypo-asm" }
 hypo-hydrate = { module = "dev.denwav.hypo:hypo-hydrate" }
 hypo-asm-hydrate = { module = "dev.denwav.hypo:hypo-asm-hydrate" }
+hypo-mappings = { module = "dev.denwav.hypo:hypo-mappings" }
+
+lorenz = "org.cadixdev:lorenz:0.5.8"
 
 feather-core = { module = "org.parchmentmc:feather", version.ref = "feather" }
 feather-gson = { module = "org.parchmentmc.feather:io-gson", version.ref = "feather" }
@@ -63,4 +66,4 @@ mockito-junit = { module = "org.mockito:mockito-junit-jupiter", version.ref = "m
 
 [bundles]
 asm = ["asm", "asm-util", "asm-tree"]
-hypo = ["hypo-model", "hypo-core", "hypo-asm", "hypo-hydrate", "hypo-asm-hydrate"]
+hypo = ["hypo-model", "hypo-core", "hypo-asm", "hypo-hydrate", "hypo-asm-hydrate", "hypo-mappings"]
diff --git a/src/main/java/io/papermc/codebook/CodeBook.java b/src/main/java/io/papermc/codebook/CodeBook.java
@@ -27,14 +27,18 @@
 import com.google.inject.Injector;
 import com.google.inject.Module;
 import com.google.inject.util.Providers;
+import dev.denwav.hypo.asm.AsmOutputWriter;
+import dev.denwav.hypo.core.HypoContext;
 import io.papermc.codebook.config.CodeBookClasspathResource;
 import io.papermc.codebook.config.CodeBookContext;
 import io.papermc.codebook.config.CodeBookJarInput;
 import io.papermc.codebook.config.CodeBookResource;
+import io.papermc.codebook.exceptions.UnexpectedException;
 import io.papermc.codebook.exceptions.UserErrorException;
 import io.papermc.codebook.pages.CodeBookPage;
 import io.papermc.codebook.pages.ExtractVanillaJarPage;
 import io.papermc.codebook.pages.FixJarPage;
+import io.papermc.codebook.pages.InspectJarPage;
 import io.papermc.codebook.pages.RemapJarPage;
 import io.papermc.codebook.pages.RemapLvtPage;
 import io.papermc.codebook.pages.UnpickPage;
@@ -71,16 +75,25 @@ private void exec(final Path tempDir) {
         final var book = List.of(
                 ExtractVanillaJarPage.class,
                 RemapJarPage.class,
-                FixJarPage.class,
                 UnpickPage.class,
+                InspectJarPage.class,
+                FixJarPage.class,
                 RemapLvtPage.class);
 
         Module module = this.createInitialModule(tempDir);
         for (final var page : book) {
             module = injector(module).getInstance(page).exec(module);
         }
 
-        final Path resultJar = injector(module).getInstance(CodeBookPage.InputJar.KEY);
+        final HypoContext context = injector(module).getInstance(CodeBookPage.Hypo.KEY);
+        final Path resultJar;
+        try (context) {
+            resultJar = tempDir.resolve("final_output.jar");
+            AsmOutputWriter.to(resultJar).write(context);
+        } catch (final Exception e) {
+            throw new UnexpectedException("Failed to write output file", e);
+        }
+
         IOUtil.move(resultJar, this.ctx.outputJar());
     }
 
diff --git a/src/main/java/io/papermc/codebook/lvt/LvtNamer.java b/src/main/java/io/papermc/codebook/lvt/LvtNamer.java
@@ -29,8 +29,6 @@
 import dev.denwav.hypo.hydrate.generic.HypoHydration;
 import dev.denwav.hypo.hydrate.generic.MethodClosure;
 import dev.denwav.hypo.model.data.ClassData;
-import dev.denwav.hypo.model.data.ClassKind;
-import dev.denwav.hypo.model.data.FieldData;
 import dev.denwav.hypo.model.data.HypoKey;
 import dev.denwav.hypo.model.data.MethodData;
 import dev.denwav.hypo.model.data.types.JvmType;
@@ -40,25 +38,29 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.cadixdev.lorenz.MappingSet;
+import org.cadixdev.lorenz.model.Mapping;
+import org.cadixdev.lorenz.model.MethodMapping;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.objectweb.asm.tree.LocalVariableNode;
 import org.objectweb.asm.tree.MethodNode;
 import org.objectweb.asm.tree.ParameterNode;
-import org.parchmentmc.feather.mapping.MappingDataContainer;
 
 public class LvtNamer {
 
     public static final HypoKey<Set<String>> SCOPED_NAMES = HypoKey.create("Scoped Names");
 
-    private final MappingDataContainer mappings;
     private final HypoContext context;
+    private final MappingSet mappings;
     private final LvtSuggester lvtSuggester;
+
     public final Map<String, AtomicInteger> missedNameSuggestions = new ConcurrentHashMap<>();
 
-    public LvtNamer(final HypoContext context, final MappingDataContainer mappings) throws IOException {
+    public LvtNamer(final HypoContext context, final MappingSet mappings) throws IOException {
         this.mappings = mappings;
         this.context = context;
         this.lvtSuggester = new LvtSuggester(context, this.missedNameSuggestions);
@@ -74,6 +76,10 @@ public void fillNames(final MethodData method) throws IOException {
         final @Nullable Set<String> names = method.get(SCOPED_NAMES);
         if (names != null) {
             // If scoped names is already filled out, this method has already been visited
+            // We don't need to be concerned with a single thread being processed by multiple threads,
+            // it can happen, but that will simply result in the same output, which is still consistent.
+            // This is fast enough that a little bit of duplicate work is acceptable, not worth trying
+            // to prevent it.
             return;
         }
 
@@ -131,13 +137,9 @@ public void fillNames(final MethodData method) throws IOException {
             this.fillNames(outerMethod);
         }
 
-        final MappingDataContainer.@Nullable MethodData methodMapping;
-        final MappingDataContainer.@Nullable ClassData classMapping = this.mappings.getClass(parentClass.name());
-        if (classMapping == null) {
-            methodMapping = null;
-        } else {
-            methodMapping = classMapping.getMethod(method.name(), method.descriptorText());
-        }
+        final Optional<MethodMapping> methodMapping = this.mappings
+                .getClassMapping(parentClass.name())
+                .flatMap(c -> c.getMethodMapping(method.name(), method.descriptorText()));
 
         // We inherit names from our outer scope, if it exists. These names will be included in our scope for any
         // potential inner scopes (other nested lambdas or local classes) that are present in this method too
@@ -162,12 +164,12 @@ public void fillNames(final MethodData method) throws IOException {
 
             for (int i = 0; i < paramCount; i++) {
                 // always (i + 1) because abstract methods are never static
-                final MappingDataContainer.@Nullable ParameterData paramMapping =
-                        methodMapping != null ? methodMapping.getParameter((byte) (i + 1)) : null;
-                @Nullable String paramName = null;
-                if (paramMapping != null) {
-                    paramName = paramMapping.getName();
-                }
+                final int fi = i;
+                @Nullable
+                String paramName = methodMapping
+                        .flatMap(m -> m.getParameterMapping(fi + 1))
+                        .map(Mapping::getDeobfuscatedName)
+                        .orElse(null);
 
                 if (paramName == null) {
                     paramName = LvtTypeSuggester.suggestNameFromType(this.context, paramTypes.get(i));
@@ -267,31 +269,18 @@ public void fillNames(final MethodData method) throws IOException {
                 }
             }
 
+            final @Nullable String paramName = methodMapping
+                    .flatMap(m -> m.getParameterMapping(lvt.index))
+                    .map(Mapping::getDeobfuscatedName)
+                    .orElse(null);
+
             @Nullable String mappedName = null;
-            if (methodMapping != null) {
-                final MappingDataContainer.@Nullable ParameterData paramMapping =
-                        methodMapping.getParameter((byte) lvt.index);
-                if (paramMapping != null) {
-                    final @Nullable String paramName = paramMapping.getName();
-                    if (paramName != null) {
-                        mappedName = LvtSuggester.determineFinalName(paramName, scopedNames);
-                    }
-                }
+            if (paramName != null) {
+                mappedName = LvtSuggester.determineFinalName(paramName, scopedNames);
             }
 
-            final String selectedName;
-            if (mappedName != null) {
-                selectedName = mappedName;
-            } else {
-                @Nullable String name = null;
-                if (parentClass.kind() == ClassKind.RECORD && method.name().equals("<init>")) {
-                    name = this.remapRecordParameter(lvt, method);
-                }
-                if (name == null) {
-                    name = this.lvtSuggester.suggestName(node, lvt, scopedNames);
-                }
-                selectedName = name;
-            }
+            final String selectedName =
+                    mappedName != null ? mappedName : this.lvtSuggester.suggestName(node, lvt, scopedNames);
 
             lvt.name = selectedName;
             usedNames[usedNameIndex++] = new UsedLvtName(lvt.name, lvt.desc, lvt.index);
@@ -308,30 +297,6 @@ public void fillNames(final MethodData method) throws IOException {
 
     private record UsedLvtName(String name, String desc, int index) {}
 
-    private @Nullable String remapRecordParameter(final LocalVariableNode lvt, final MethodData method) {
-        // use record component names for primary constructor
-        final int paramIndex = fromLvtToParamIndex(lvt.index, method);
-        if (paramIndex == -1) {
-            return null;
-        }
-        final @Nullable List<FieldData> comp = method.parentClass().recordComponents();
-        if (comp == null) {
-            throw new IllegalStateException("No record components found on record");
-        }
-
-        if (comp.size() != method.params().size()) {
-            return null;
-        } else {
-            for (int i = 0; i < comp.size(); i++) {
-                if (!comp.get(i).fieldType().equals(method.param(i))) {
-                    return null;
-                }
-            }
-        }
-
-        return comp.get(paramIndex).name();
-    }
-
     private static int find(final int[] array, final int value) {
         return find(array, value, array.length);
     }
diff --git a/src/main/java/io/papermc/codebook/pages/CodeBookPage.java b/src/main/java/io/papermc/codebook/pages/CodeBookPage.java
@@ -29,6 +29,7 @@
 import com.google.inject.binder.LinkedBindingBuilder;
 import com.google.inject.util.Modules;
 import com.google.inject.util.Providers;
+import dev.denwav.hypo.core.HypoContext;
 import io.papermc.codebook.config.CodeBookContext;
 import jakarta.inject.Qualifier;
 import java.lang.annotation.ElementType;
@@ -38,6 +39,7 @@
 import java.nio.file.Path;
 import java.util.IdentityHashMap;
 import java.util.List;
+import org.cadixdev.lorenz.MappingSet;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 public abstract class CodeBookPage {
@@ -124,6 +126,7 @@ public void to(final @Nullable T value) {
     @Retention(RetentionPolicy.RUNTIME)
     public @interface ParamMappings {
         Key<Path> PATH_KEY = Key.get(Path.class, ParamMappings.class);
+        Key<MappingSet> KEY = Key.get(MappingSet.class, ParamMappings.class);
     }
 
     @Qualifier
@@ -146,4 +149,11 @@ public void to(final @Nullable T value) {
     public @interface TempDir {
         Key<Path> KEY = Key.get(Path.class, TempDir.class);
     }
+
+    @Qualifier
+    @Target(ElementType.PARAMETER)
+    @Retention(RetentionPolicy.RUNTIME)
+    public @interface Hypo {
+        Key<HypoContext> KEY = Key.get(HypoContext.class, Hypo.class);
+    }
 }
diff --git a/src/main/java/io/papermc/codebook/pages/FixJarPage.java b/src/main/java/io/papermc/codebook/pages/FixJarPage.java
@@ -26,17 +26,11 @@
 
 import com.google.common.collect.Iterables;
 import dev.denwav.hypo.asm.AsmClassData;
-import dev.denwav.hypo.asm.AsmClassDataProvider;
 import dev.denwav.hypo.asm.AsmConstructorData;
 import dev.denwav.hypo.asm.AsmFieldData;
 import dev.denwav.hypo.asm.AsmMethodData;
-import dev.denwav.hypo.asm.AsmOutputWriter;
-import dev.denwav.hypo.asm.hydrate.BridgeMethodHydrator;
 import dev.denwav.hypo.core.HypoContext;
-import dev.denwav.hypo.hydrate.HydrationManager;
 import dev.denwav.hypo.hydrate.generic.HypoHydration;
-import dev.denwav.hypo.model.ClassProviderRoot;
-import dev.denwav.hypo.model.HypoModelUtil;
 import dev.denwav.hypo.model.data.ClassData;
 import dev.denwav.hypo.model.data.ClassKind;
 import dev.denwav.hypo.model.data.FieldData;
@@ -45,9 +39,10 @@
 import io.papermc.codebook.exceptions.UnexpectedException;
 import jakarta.inject.Inject;
 import java.io.IOException;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AnnotationNode;
@@ -56,58 +51,40 @@
 
 public final class FixJarPage extends CodeBookPage {
 
-    private final Path inputJar;
-    private final List<Path> classpath;
-    private final Path tempDir;
+    private final HypoContext context;
 
     @Inject
-    public FixJarPage(
-            @InputJar final Path inputJar, @ClasspathJars final List<Path> classpath, @TempDir final Path tempDir) {
-        this.inputJar = inputJar;
-        this.classpath = classpath;
-        this.tempDir = tempDir;
+    public FixJarPage(@Hypo final HypoContext context) {
+        this.context = context;
     }
 
     @Override
     public void exec() {
-        final HypoContext context;
         try {
-            context = this.createContext();
+            this.fixJar();
         } catch (final IOException e) {
-            throw new UnexpectedException("Failed to create context for bytecode analysis", e);
-        }
-
-        try (context) {
-            HydrationManager.createDefault()
-                    .register(BridgeMethodHydrator.create())
-                    .hydrate(context);
-
-            final Path result = this.fixWithContext(context);
-            this.bind(InputJar.KEY).to(result);
-        } catch (final Exception e) {
             throw new UnexpectedException("Failed to fix jar", e);
         }
     }
 
-    private HypoContext createContext() throws IOException {
-        return HypoContext.builder()
-                .withProvider(AsmClassDataProvider.of(ClassProviderRoot.fromJar(this.inputJar)))
-                .withContextProvider(AsmClassDataProvider.of(this.classpath.stream()
-                        .map(HypoModelUtil.wrapFunction(ClassProviderRoot::fromJar))
-                        .toList()))
-                .withContextProvider(AsmClassDataProvider.of(ClassProviderRoot.ofJdk()))
-                .build();
-    }
-
-    private Path fixWithContext(final HypoContext context) throws IOException {
-        for (final ClassData classData : context.getProvider().allClasses()) {
-            this.processClass((AsmClassData) classData);
+    private void fixJar() throws IOException {
+        final var tasks = new ArrayList<Future<?>>();
+        for (final ClassData classData : this.context.getProvider().allClasses()) {
+            final var task = this.context.getExecutor().submit(() -> {
+                this.processClass((AsmClassData) classData);
+            });
+            tasks.add(task);
         }
 
-        final Path fixedJar = this.tempDir.resolve("fixed.jar");
-        AsmOutputWriter.to(fixedJar).write(context);
-
-        return fixedJar;
+        try {
+            for (final Future<?> task : tasks) {
+                task.get();
+            }
+        } catch (final ExecutionException e) {
+            throw new UnexpectedException("Failed to fix jar", e.getCause());
+        } catch (final InterruptedException e) {
+            throw new UnexpectedException("Jar fixing interrupted", e);
+        }
     }
 
     private void processClass(final AsmClassData classData) {
diff --git a/src/main/java/io/papermc/codebook/pages/InspectJarPage.java b/src/main/java/io/papermc/codebook/pages/InspectJarPage.java
diff --git a/src/main/java/io/papermc/codebook/pages/RemapLvtPage.java b/src/main/java/io/papermc/codebook/pages/RemapLvtPage.java

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`group = io.papermc.codebook`
`2`		`-version = 1.0.3-SNAPSHOT`
	`2`	`+version = 1.0.3`