From 56375487244fa1f14d2414cc91aa3a9c301b68e5 Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 19 Nov 2023 22:20:10 +0100 Subject: [PATCH 1/4] Retrieve jar module descriptors in parallel --- .../java/cpw/mods/cl/JarModuleFinder.java | 8 ++-- .../cpw/mods/jarhandling/JarMetadata.java | 33 ++++++++----- .../cpw/mods/jarhandling/LazyJarMetadata.java | 28 +++++++++++ .../jarhandling/impl/ModuleJarMetadata.java | 46 +++++++++++++------ .../jarhandling/impl/SimpleJarMetadata.java | 32 +++++++++++-- 5 files changed, 115 insertions(+), 32 deletions(-) create mode 100644 src/main/java/cpw/mods/jarhandling/LazyJarMetadata.java diff --git a/src/main/java/cpw/mods/cl/JarModuleFinder.java b/src/main/java/cpw/mods/cl/JarModuleFinder.java index bc4128d..a22e015 100644 --- a/src/main/java/cpw/mods/cl/JarModuleFinder.java +++ b/src/main/java/cpw/mods/cl/JarModuleFinder.java @@ -21,10 +21,12 @@ public class JarModuleFinder implements ModuleFinder { private final Map moduleReferenceMap; JarModuleFinder(final SecureJar... jars) { - record ref(SecureJar.ModuleDataProvider jar, ModuleReference ref) {} this.moduleReferenceMap = Arrays.stream(jars) - .map(jar->new ref(jar.moduleDataProvider(), new JarModuleReference(jar.moduleDataProvider()))) - .collect(Collectors.toMap(r->r.jar.name(), r->r.ref, (r1, r2)->r1)); + // Computing the module descriptor can be slow so do it in parallel! + // Jars are not thread safe internally, but they are independent, so this is safe. + .parallel() + // Note: Collectors.toMap() works fine with parallel streams. + .collect(Collectors.toMap(jar -> jar.moduleDataProvider().name(), jar -> new JarModuleReference(jar.moduleDataProvider()), (r1, r2) -> r1)); } @Override diff --git a/src/main/java/cpw/mods/jarhandling/JarMetadata.java b/src/main/java/cpw/mods/jarhandling/JarMetadata.java index 7a8eda5..9e19669 100644 --- a/src/main/java/cpw/mods/jarhandling/JarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/JarMetadata.java @@ -2,15 +2,18 @@ import cpw.mods.jarhandling.impl.ModuleJarMetadata; import cpw.mods.jarhandling.impl.SimpleJarMetadata; +import org.jetbrains.annotations.Nullable; import java.lang.module.ModuleDescriptor; import java.nio.file.Path; import java.util.List; import java.util.Set; +import java.util.function.Supplier; import java.util.regex.Pattern; public interface JarMetadata { String name(); + @Nullable String version(); ModuleDescriptor descriptor(); // ALL from jdk.internal.module.ModulePath.java @@ -41,23 +44,23 @@ public interface JarMetadata { * from {@code Automatic-Module-Name} in the manifest. */ static JarMetadata from(JarContents jar) { - final var pkgs = jar.getPackages(); var mi = jar.findFile("module-info.class"); if (mi.isPresent()) { - return new ModuleJarMetadata(mi.get(), pkgs); + return new ModuleJarMetadata(mi.get(), jar::getPackages); } else { var providers = jar.getMetaInfServices(); - var fileCandidate = fromFileName(jar.getPrimaryPath(), pkgs, providers); + Supplier> packagesSupplier = jar::getPackages; + var fileCandidate = fromFileName(jar.getPrimaryPath(), packagesSupplier, providers); var aname = jar.getManifest().getMainAttributes().getValue("Automatic-Module-Name"); if (aname != null) { - return new SimpleJarMetadata(aname, fileCandidate.version(), pkgs, providers); + return new SimpleJarMetadata(aname, fileCandidate.version(), packagesSupplier, providers); } else { return fileCandidate; } } } - static SimpleJarMetadata fromFileName(final Path path, final Set pkgs, final List providers) { + static SimpleJarMetadata fromFileName(final Path path, Supplier> packagesSupplier, final List providers) { // detect Maven-like paths Path versionMaybe = path.getParent(); if (versionMaybe != null) @@ -73,9 +76,9 @@ static SimpleJarMetadata fromFileName(final Path path, final Set pkgs, f if (mat.find()) { var potential = ver.substring(mat.start()); ver = safeParseVersion(potential, path.getFileName().toString()); - return new SimpleJarMetadata(cleanModuleName(name), ver, pkgs, providers); + return new SimpleJarMetadata(cleanModuleName(name), ver, packagesSupplier, providers); } else { - return new SimpleJarMetadata(cleanModuleName(name), null, pkgs, providers); + return new SimpleJarMetadata(cleanModuleName(name), null, packagesSupplier, providers); } } } @@ -93,9 +96,9 @@ static SimpleJarMetadata fromFileName(final Path path, final Set pkgs, f var potential = fn.substring(mat.start() + 1); var ver = safeParseVersion(potential, path.getFileName().toString()); var name = mat.replaceAll(""); - return new SimpleJarMetadata(cleanModuleName(name), ver, pkgs, providers); + return new SimpleJarMetadata(cleanModuleName(name), ver, packagesSupplier, providers); } else { - return new SimpleJarMetadata(cleanModuleName(fn), null, pkgs, providers); + return new SimpleJarMetadata(cleanModuleName(fn), null, packagesSupplier, providers); } } @@ -144,6 +147,14 @@ private static String cleanModuleName(String mn) { return mn; } + /** + * @deprecated Use {@link #fromFileName(Path, Supplier, List)} instead. + */ + @Deprecated(forRemoval = true, since = "TODO when?") + static SimpleJarMetadata fromFileName(final Path path, final Set pkgs, final List providers) { + return fromFileName(path, () -> pkgs, providers); + } + /** * @deprecated Use {@link #from(JarContents)} instead. */ @@ -153,13 +164,13 @@ static JarMetadata from(final SecureJar jar, final Path... path) { final var pkgs = jar.getPackages(); var mi = jar.moduleDataProvider().findFile("module-info.class"); if (mi.isPresent()) { - return new ModuleJarMetadata(mi.get(), pkgs); + return new ModuleJarMetadata(mi.get(), jar::getPackages); } else { var providers = jar.getProviders(); var fileCandidate = fromFileName(path[0], pkgs, providers); var aname = jar.moduleDataProvider().getManifest().getMainAttributes().getValue("Automatic-Module-Name"); if (aname != null) { - return new SimpleJarMetadata(aname, fileCandidate.version(), pkgs, providers); + return new SimpleJarMetadata(aname, fileCandidate.version(), () -> pkgs, providers); } else { return fileCandidate; } diff --git a/src/main/java/cpw/mods/jarhandling/LazyJarMetadata.java b/src/main/java/cpw/mods/jarhandling/LazyJarMetadata.java new file mode 100644 index 0000000..49b6b4b --- /dev/null +++ b/src/main/java/cpw/mods/jarhandling/LazyJarMetadata.java @@ -0,0 +1,28 @@ +package cpw.mods.jarhandling; + +import org.jetbrains.annotations.Nullable; + +import java.lang.module.ModuleDescriptor; + +/** + * Base class for {@link JarMetadata} implementations that lazily compute their descriptor. + * This is recommended because descriptor computation can then run in parallel. + */ +public abstract class LazyJarMetadata implements JarMetadata { + @Nullable + private ModuleDescriptor descriptor; + + @Override + public final ModuleDescriptor descriptor() { + if (descriptor == null) { + descriptor = computeDescriptor(); + } + return descriptor; + } + + /** + * Computes the module descriptor for this jar. + * This method is called at most once. + */ + protected abstract ModuleDescriptor computeDescriptor(); +} diff --git a/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java index 9956d13..ba05087 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/ModuleJarMetadata.java @@ -1,6 +1,8 @@ package cpw.mods.jarhandling.impl; import cpw.mods.jarhandling.JarMetadata; +import cpw.mods.jarhandling.LazyJarMetadata; +import org.jetbrains.annotations.Nullable; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ModuleVisitor; @@ -15,29 +17,42 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Set; +import java.util.function.Supplier; /** * {@link JarMetadata} implementation for a modular jar. * Reads the module descriptor from the jar. */ -public class ModuleJarMetadata implements JarMetadata { - private final ModuleDescriptor descriptor; +public class ModuleJarMetadata extends LazyJarMetadata { + private final ModuleClassVisitor mcv; + private final Supplier> packagesSupplier; - public ModuleJarMetadata(final URI uri, final Set packages) { + public ModuleJarMetadata(URI uri, Supplier> packagesSupplier) { try (var is = Files.newInputStream(Path.of(uri))) { ClassReader cr = new ClassReader(is); var mcv = new ModuleClassVisitor(); cr.accept(mcv, ClassReader.SKIP_CODE); - mcv.mfv().packages().addAll(packages); - mcv.mfv().builder().packages(mcv.mfv.packages()); - descriptor = mcv.mfv().builder().build(); + this.mcv = mcv; } catch (IOException e) { throw new UncheckedIOException(e); } + + // Defer package scanning until computeDescriptor() + this.packagesSupplier = packagesSupplier; } - private class ModuleClassVisitor extends ClassVisitor { + @Override + protected ModuleDescriptor computeDescriptor() { + mcv.mfv().packages().addAll(packagesSupplier.get()); + mcv.mfv().builder().packages(mcv.mfv().packages()); + return mcv.mfv().builder().build(); + } + + private static class ModuleClassVisitor extends ClassVisitor { private ModFileVisitor mfv; + private String name; + @Nullable + private String version; ModuleClassVisitor() { super(Opcodes.ASM9); @@ -46,6 +61,8 @@ private class ModuleClassVisitor extends ClassVisitor { @Override public ModuleVisitor visitModule(final String name, final int access, final String version) { this.mfv = new ModFileVisitor(name, access, version); + this.name = name; + this.version = version; return this.mfv; } @@ -53,12 +70,14 @@ public ModFileVisitor mfv() { return mfv; } } - private class ModFileVisitor extends ModuleVisitor { + + private static class ModFileVisitor extends ModuleVisitor { private final ModuleDescriptor.Builder builder; private final Set packages = new HashSet<>(); public ModFileVisitor(final String name, final int access, final String version) { super(Opcodes.ASM9); + // Note: we ignore the access flags and make every module open instead! builder = ModuleDescriptor.newOpenModule(name); if (version != null) builder.version(version); } @@ -127,18 +146,15 @@ public Set packages() { return packages; } } + @Override public String name() { - return descriptor.name(); + return mcv.name; } @Override + @Nullable public String version() { - return descriptor.version().toString(); - } - - @Override - public ModuleDescriptor descriptor() { - return descriptor; + return mcv.version; } } diff --git a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java index 1ded493..c99f039 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java @@ -1,22 +1,48 @@ package cpw.mods.jarhandling.impl; import cpw.mods.jarhandling.JarMetadata; +import cpw.mods.jarhandling.LazyJarMetadata; import cpw.mods.jarhandling.SecureJar; +import org.jetbrains.annotations.Nullable; import java.lang.module.ModuleDescriptor; import java.util.List; import java.util.Set; +import java.util.function.Supplier; /** * {@link JarMetadata} implementation for a non-modular jar, turning it into an automatic module. */ -public record SimpleJarMetadata(String name, String version, Set pkgs, List providers) implements JarMetadata { +public class SimpleJarMetadata extends LazyJarMetadata implements JarMetadata { + private final String name; + private final String version; + private final Supplier> packagesSupplier; + private final List providers; + + public SimpleJarMetadata(String name, String version, Supplier> packagesSupplier, List providers) { + this.name = name; + this.version = version; + this.packagesSupplier = packagesSupplier; + this.providers = providers; + } + + @Override + public String name() { + return name; + } + + @Override + @Nullable + public String version() { + return version; + } + @Override - public ModuleDescriptor descriptor() { + public ModuleDescriptor computeDescriptor() { var bld = ModuleDescriptor.newAutomaticModule(name()); if (version()!=null) bld.version(version()); - bld.packages(pkgs()); + bld.packages(packagesSupplier.get()); providers.stream().filter(p->!p.providers().isEmpty()).forEach(p->bld.provides(p.serviceName(), p.providers())); return bld.build(); } From 72c99b78b920e55f1ad3356eb4a37fc6da21752f Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 26 Nov 2023 22:41:14 +0100 Subject: [PATCH 2/4] Cleanup name and version handling --- .../cpw/mods/jarhandling/JarMetadata.java | 39 ++++++++++--------- .../cpw/mods/jarhandling/NameAndVersion.java | 9 +++++ 2 files changed, 29 insertions(+), 19 deletions(-) create mode 100644 src/main/java/cpw/mods/jarhandling/NameAndVersion.java diff --git a/src/main/java/cpw/mods/jarhandling/JarMetadata.java b/src/main/java/cpw/mods/jarhandling/JarMetadata.java index 9e19669..6e3a31d 100644 --- a/src/main/java/cpw/mods/jarhandling/JarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/JarMetadata.java @@ -8,7 +8,6 @@ import java.nio.file.Path; import java.util.List; import java.util.Set; -import java.util.function.Supplier; import java.util.regex.Pattern; public interface JarMetadata { @@ -48,19 +47,20 @@ static JarMetadata from(JarContents jar) { if (mi.isPresent()) { return new ModuleJarMetadata(mi.get(), jar::getPackages); } else { - var providers = jar.getMetaInfServices(); - Supplier> packagesSupplier = jar::getPackages; - var fileCandidate = fromFileName(jar.getPrimaryPath(), packagesSupplier, providers); - var aname = jar.getManifest().getMainAttributes().getValue("Automatic-Module-Name"); - if (aname != null) { - return new SimpleJarMetadata(aname, fileCandidate.version(), packagesSupplier, providers); - } else { - return fileCandidate; + var nav = computeNameAndVersion(jar.getPrimaryPath()); + String name = nav.name(); + String version = nav.version(); + + String automaticModuleName = jar.getManifest().getMainAttributes().getValue("Automatic-Module-Name"); + if (automaticModuleName != null) { + name = automaticModuleName; } + + return new SimpleJarMetadata(name, version, jar::getPackages, jar.getMetaInfServices()); } } - static SimpleJarMetadata fromFileName(final Path path, Supplier> packagesSupplier, final List providers) { + private static NameAndVersion computeNameAndVersion(Path path) { // detect Maven-like paths Path versionMaybe = path.getParent(); if (versionMaybe != null) @@ -76,29 +76,29 @@ static SimpleJarMetadata fromFileName(final Path path, Supplier> pac if (mat.find()) { var potential = ver.substring(mat.start()); ver = safeParseVersion(potential, path.getFileName().toString()); - return new SimpleJarMetadata(cleanModuleName(name), ver, packagesSupplier, providers); + return new NameAndVersion(cleanModuleName(name), ver); } else { - return new SimpleJarMetadata(cleanModuleName(name), null, packagesSupplier, providers); + return new NameAndVersion(cleanModuleName(name), null); } } } } // fallback parsing - var fn = path.getFileName().toString(); + var fn = path.getFileName().toString(); var lastDot = fn.lastIndexOf('.'); if (lastDot > 0) { fn = fn.substring(0, lastDot); // strip extension if possible } - + var mat = DASH_VERSION.matcher(fn); if (mat.find()) { var potential = fn.substring(mat.start() + 1); var ver = safeParseVersion(potential, path.getFileName().toString()); var name = mat.replaceAll(""); - return new SimpleJarMetadata(cleanModuleName(name), ver, packagesSupplier, providers); + return new NameAndVersion(cleanModuleName(name), ver); } else { - return new SimpleJarMetadata(cleanModuleName(fn), null, packagesSupplier, providers); + return new NameAndVersion(cleanModuleName(fn), null); } } @@ -148,11 +148,12 @@ private static String cleanModuleName(String mn) { } /** - * @deprecated Use {@link #fromFileName(Path, Supplier, List)} instead. + * @deprecated Build from jar contents directly using {@link #from(JarContents)}. */ - @Deprecated(forRemoval = true, since = "TODO when?") + @Deprecated(forRemoval = true, since = "2.1.23") static SimpleJarMetadata fromFileName(final Path path, final Set pkgs, final List providers) { - return fromFileName(path, () -> pkgs, providers); + var nav = computeNameAndVersion(path); + return new SimpleJarMetadata(nav.name(), nav.version(), () -> pkgs, providers); } /** diff --git a/src/main/java/cpw/mods/jarhandling/NameAndVersion.java b/src/main/java/cpw/mods/jarhandling/NameAndVersion.java new file mode 100644 index 0000000..bbf2d18 --- /dev/null +++ b/src/main/java/cpw/mods/jarhandling/NameAndVersion.java @@ -0,0 +1,9 @@ +package cpw.mods.jarhandling; + +import org.jetbrains.annotations.Nullable; + +/** + * Holder class for name and version of a module, used in {@link JarMetadata} computations. + * Unfortunately, interfaces cannot hold private records (yet?). + */ +record NameAndVersion(String name, @Nullable String version) {} From 9483fb2f1b03ba45a2dfdd03f2ed9393278b3c55 Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 26 Nov 2023 22:57:55 +0100 Subject: [PATCH 3/4] Add more efficient way to query service providers --- src/main/java/cpw/mods/jarhandling/JarMetadata.java | 11 +++++++++++ .../cpw/mods/jarhandling/impl/SimpleJarMetadata.java | 9 ++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/cpw/mods/jarhandling/JarMetadata.java b/src/main/java/cpw/mods/jarhandling/JarMetadata.java index 6e3a31d..2302297 100644 --- a/src/main/java/cpw/mods/jarhandling/JarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/JarMetadata.java @@ -15,6 +15,17 @@ public interface JarMetadata { @Nullable String version(); ModuleDescriptor descriptor(); + + /** + * {@return the provider declarations for this jar} + * + *

Computing the {@link #descriptor()} can be expensive as it requires scanning the jar for packages. + * If only the service providers are needed, this method can be used instead. + */ + default List providers() { + return descriptor().provides().stream().map(p -> new SecureJar.Provider(p.service(), p.providers())).toList(); + } + // ALL from jdk.internal.module.ModulePath.java Pattern DASH_VERSION = Pattern.compile("-([.\\d]+)"); Pattern NON_ALPHANUM = Pattern.compile("[^A-Za-z0-9]"); diff --git a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java index c99f039..c86f967 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java @@ -6,6 +6,7 @@ import org.jetbrains.annotations.Nullable; import java.lang.module.ModuleDescriptor; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.function.Supplier; @@ -24,6 +25,7 @@ public SimpleJarMetadata(String name, String version, Supplier> pack this.version = version; this.packagesSupplier = packagesSupplier; this.providers = providers; + this.providers.removeIf(p -> p.providers().isEmpty()); } @Override @@ -43,7 +45,12 @@ public ModuleDescriptor computeDescriptor() { if (version()!=null) bld.version(version()); bld.packages(packagesSupplier.get()); - providers.stream().filter(p->!p.providers().isEmpty()).forEach(p->bld.provides(p.serviceName(), p.providers())); + providers.forEach(p->bld.provides(p.serviceName(), p.providers())); return bld.build(); } + + @Override + public List providers() { + return Collections.unmodifiableList(providers); + } } From e8829d1f1def319d230191aabfe395f028e119e1 Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 26 Nov 2023 23:25:20 +0100 Subject: [PATCH 4/4] Fix accidentally modifying an unmodifiable list --- src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java index c86f967..eb56fb0 100644 --- a/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java +++ b/src/main/java/cpw/mods/jarhandling/impl/SimpleJarMetadata.java @@ -24,8 +24,7 @@ public SimpleJarMetadata(String name, String version, Supplier> pack this.name = name; this.version = version; this.packagesSupplier = packagesSupplier; - this.providers = providers; - this.providers.removeIf(p -> p.providers().isEmpty()); + this.providers = providers.stream().filter(p -> !p.providers().isEmpty()).toList(); } @Override