From 7d71f6220d47abc85889ccde406999e22c758a76 Mon Sep 17 00:00:00 2001 From: Sam Snyder Date: Wed, 7 Aug 2024 16:40:29 -0700 Subject: [PATCH] Remove ClassGraph from computation of JavaSourceSet, include type to GAV coordinate mappings (#4391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Simplify JavaSourceSet calculation by removing ClassGraph. We've given up ever fully hydrating the type information in this marker, so the more advanced features of ClassGraph are unnecessary. * Include mappings of types -> gav and gav -> types to facilitate the authoring of recipes which eliminate unused dependencies. * Add benchmark comparing new and old JavaSourceSet implementations. * No static state --------- Co-authored-by: Jonathan Schnéider --- .../java/JavaSourceSetBenchmark.java | 38 ++++ .../java/marker/JavaSourceSetTest.java | 94 +++------- .../java/org/openrewrite/java/Assertions.java | 2 +- .../java/marker/JavaSourceSet.java | 177 ++++++++++++++++-- settings.gradle.kts | 10 +- 5 files changed, 231 insertions(+), 90 deletions(-) create mode 100644 rewrite-benchmarks/src/jmh/java/org/openrewrite/benchmarks/java/JavaSourceSetBenchmark.java diff --git a/rewrite-benchmarks/src/jmh/java/org/openrewrite/benchmarks/java/JavaSourceSetBenchmark.java b/rewrite-benchmarks/src/jmh/java/org/openrewrite/benchmarks/java/JavaSourceSetBenchmark.java new file mode 100644 index 00000000000..e21aab4dd12 --- /dev/null +++ b/rewrite-benchmarks/src/jmh/java/org/openrewrite/benchmarks/java/JavaSourceSetBenchmark.java @@ -0,0 +1,38 @@ +package org.openrewrite.benchmarks.java; + +import org.openjdk.jmh.annotations.*; +import org.openrewrite.java.JavaParser; +import org.openrewrite.java.internal.JavaTypeCache; +import org.openrewrite.java.marker.JavaSourceSet; + +import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.TimeUnit; + +@State(Scope.Benchmark) +@Fork(1) +@Measurement(iterations = 2) +@Warmup(iterations = 2) +@BenchmarkMode(Mode.SampleTime) +@OutputTimeUnit(TimeUnit.SECONDS) +@Threads(4) +public class JavaSourceSetBenchmark { + + List classpath; + + @Setup + public void setup() { + classpath = JavaParser.runtimeClasspath(); + } + + @Benchmark + public void jarIOBenchmark() { + JavaSourceSet.build("main", classpath); + } + + @Benchmark + public void classgraphBenchmark() { + //noinspection deprecation + JavaSourceSet.build("main", classpath, new JavaTypeCache(), false); + } +} diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java index d1bae6962c7..ee4ac10aba2 100755 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java @@ -15,101 +15,59 @@ */ package org.openrewrite.java.marker; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openrewrite.Issue; -import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaParser; -import org.openrewrite.java.JavaTypeGoat; -import org.openrewrite.java.JavaTypeVisitor; -import org.openrewrite.java.internal.JavaReflectionTypeMapping; -import org.openrewrite.java.internal.JavaTypeCache; import org.openrewrite.java.tree.JavaType; -import java.util.Collections; -import java.util.IdentityHashMap; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; +import java.nio.file.Paths; import java.util.function.Function; import java.util.stream.Collectors; import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Fail.fail; +import static org.openrewrite.java.marker.JavaSourceSet.gavFromPath; class JavaSourceSetTest { @Issue("https://github.com/openrewrite/rewrite/issues/1636") - @Test - void buildJavaSourceSet() { - var typeCache = new JavaTypeCache(); - var jss = JavaSourceSet.build("main", emptyList(), typeCache, false); - var typesBySignature = jss.getClasspath().stream().collect(Collectors.toMap(JavaType.FullyQualified::toString, Function.identity())); - assertThat(typesBySignature.get("java.lang.Object")).isInstanceOf(JavaType.Class.class); - assertThat(typesBySignature.get("java.util.List")).isInstanceOf(JavaType.Class.class); - } - @Issue("https://github.com/openrewrite/rewrite/issues/1712") @Test - void shallowTypes() { - var typeCache = new JavaTypeCache(); - var jss = JavaSourceSet.build("main", emptyList(), typeCache, false); + void buildJavaSourceSet() { + var jss = JavaSourceSet.build("main", emptyList()); var typesBySignature = jss.getClasspath().stream().collect(Collectors.toMap(JavaType.FullyQualified::toString, Function.identity())); - assertThat(typesBySignature.get("java.lang.Object")).isInstanceOf(JavaType.ShallowClass.class); - assertThat(typesBySignature.get("java.util.List")).isInstanceOf(JavaType.ShallowClass.class); + assertThat(typesBySignature.get("java.lang.Object")).isInstanceOf(JavaType.FullyQualified.class); + assertThat(typesBySignature.get("java.util.List")).isInstanceOf(JavaType.FullyQualified.class); } @Issue("https://github.com/openrewrite/rewrite/issues/1677") @Test void shadedJar() { - var typeCache = new JavaTypeCache(); - var shaded = JavaSourceSet.build("test", JavaParser.dependenciesFromClasspath("hbase-shaded-client"), typeCache, false) - .getClasspath().stream().filter(o -> o.getFullyQualifiedName().startsWith("org.apache.hadoop.hbase.shaded")).collect(Collectors.toList()); - assertThat(shaded).isNotEmpty(); - assertThat(shaded.get(0)).isInstanceOf(JavaType.ShallowClass.class); + JavaSourceSet jss = JavaSourceSet.build("test", JavaParser.dependenciesFromClasspath("hbase-shaded-client")); + var shaded = jss.getClasspath().stream() + .filter(o -> o.getFullyQualifiedName().startsWith("org.apache.hadoop.hbase.CacheEvictionStats")) + .findAny(); + assertThat(shaded).isPresent(); + assertThat(jss.getTypeToGav().get(shaded.get())).isEqualTo("org.apache.hbase:hbase-shaded-client:2.4.11"); } - // This test uses a lot of memory and examines the "fullTypeInformation" path that we don't actually take anywhere right now - @Disabled @Test - void doesNotDuplicateTypesInCache() { - var typeCache = new JavaTypeCache(); - Set uniqueTypes = Collections.newSetFromMap(new IdentityHashMap<>()); - var reflectiveGoat = new JavaReflectionTypeMapping(typeCache).type(JavaTypeGoat.class); - newUniqueTypes(uniqueTypes, reflectiveGoat, false); - - var classpathGoat = JavaSourceSet.build("main", JavaParser.runtimeClasspath(), typeCache, true) - .getClasspath() - .stream() - .filter(t -> t.getClassName().equals("JavaTypeGoat")) - .findAny() - .orElseThrow(() -> new IllegalStateException("Could not find JavaTypeGoat in classpath")); - - newUniqueTypes(uniqueTypes, classpathGoat, true); + void runtimeClasspath() { + var jss = JavaSourceSet.build("main", JavaParser.runtimeClasspath()).getClasspath() + .stream().filter(it -> it.getFullyQualifiedName().contains("org.openrewrite")) + .toList(); + assertThat(jss).isNotEmpty(); } - private void newUniqueTypes(Set uniqueTypes, JavaType root, boolean report) { - var newUnique = new AtomicBoolean(false); - - new JavaTypeVisitor() { - @Override - public JavaType visit(@Nullable JavaType javaType, Integer p) { - if (javaType != null) { - if (uniqueTypes.add(javaType)) { - if (report) { - newUnique.set(true); - System.out.println(javaType); - } - return super.visit(javaType, p); - } - } - //noinspection ConstantConditions - return null; - } - }.visit(root, 0); + @Test + void gavCoordinateFromGradle() { + assertThat(gavFromPath(Paths.get("C:/Users/Sam/.gradle/caches/modules-2/files-2.1/org.openrewrite/rewrite-core/8.32.0/64ddcc371f1bf29593b4b27e907757d5554d1a83/rewrite-core-8.32.0.jar"))) + .isEqualTo("org.openrewrite:rewrite-core:8.32.0"); + } - if (report && newUnique.get()) { - fail("Found new unique types there should have been none."); - } + @Test + void gavCoordinateFromMaven() { + assertThat(gavFromPath(Paths.get("C:/Users/Sam/.m2/repository/org/openrewrite/rewrite-core/8.32.0/rewrite-core-8.32.0.jar"))) + .isEqualTo("org.openrewrite:rewrite-core:8.32.0"); } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/Assertions.java b/rewrite-java/src/main/java/org/openrewrite/java/Assertions.java index 0930a3b5e47..673970be59d 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/Assertions.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/Assertions.java @@ -233,6 +233,6 @@ private static JavaProject javaProject(String projectName) { private static JavaSourceSet javaSourceSet(String sourceSet) { return javaSourceSets.computeIfAbsent(sourceSet, name -> - new JavaSourceSet(Tree.randomId(), name, Collections.emptyList())); + new JavaSourceSet(Tree.randomId(), name, Collections.emptyList(), Collections.emptyMap(), Collections.emptyMap())); } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/marker/JavaSourceSet.java b/rewrite-java/src/main/java/org/openrewrite/java/marker/JavaSourceSet.java index 08337429d70..3196b5c62a0 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/marker/JavaSourceSet.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/marker/JavaSourceSet.java @@ -22,16 +22,18 @@ import lombok.EqualsAndHashCode; import lombok.Value; import lombok.With; +import org.openrewrite.PathUtils; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.internal.JavaTypeCache; import org.openrewrite.java.tree.JavaType; import org.openrewrite.marker.SourceSet; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.UUID; +import java.io.IOException; +import java.net.URI; +import java.nio.file.*; +import java.util.*; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; import static org.openrewrite.Tree.randomId; @@ -46,12 +48,28 @@ public class JavaSourceSet implements SourceSet { List classpath; + /** + * Mapping of a String taking the form "group:artifact:version" to the types provided by that artifact. + * Does not include java standard library types. + */ + Map> gavToTypes; + + /** + * Mapping of a JavaType to the String "group:artifact:version" of the artifact that provides that type. + * Does not include java standard library types. + */ + Map typeToGav; + /** * Extract type information from the provided classpath. + * Uses ClassGraph to compute the classpath. + + * Does not support gavToTypes or typeToGav mapping * - * @param fullTypeInformation when false classpath will be filled with shallow types (effectively just fully-qualified names). - * when true a much more memory-intensive, time-consuming approach will extract full type information + * @param fullTypeInformation Not used, does not do anything, to be deleted + * @param ignore Not used, does not do anything, to be deleted */ + @Deprecated public static JavaSourceSet build(String sourceSetName, Collection classpath, JavaTypeCache ignore, boolean fullTypeInformation) { if (fullTypeInformation) { @@ -83,9 +101,10 @@ public static JavaSourceSet build(String sourceSetName, Collection classpa // Peculiarly, Classgraph will not return a ClassInfo for java.lang.Object, although it does for all other java.lang types typeNames.add("java.lang.Object"); - return new JavaSourceSet(randomId(), sourceSetName, typesFrom(typeNames)); + return new JavaSourceSet(randomId(), sourceSetName, typesFrom(typeNames), null, null); } + /* * Create a map of package names to types contained within that package. Type names are not fully qualified, except for type parameter bounds. * e.g.: "java.util" -> [List, Date] @@ -163,7 +182,7 @@ private static List typesFrom(List typeNames) { } String nameFragment = classInfo.getName().substring(sb.length()); - if (isUndeclarable(nameFragment)) { + if (!isDeclarable(nameFragment)) { return null; } sb.append(nameFragment); @@ -171,15 +190,147 @@ private static List typesFrom(List typeNames) { } else { name = classInfo.getName(); } - if (isUndeclarable(name)) { + if (!isDeclarable(name)) { return null; } return name; } - @SuppressWarnings("SpellCheckingInspection") - private static boolean isUndeclarable(String className) { + + // Purely IO-based classpath scanning below this point + /** + * Extract type information from the provided classpath. + * Uses file I/O to compute the classpath. + */ + public static JavaSourceSet build(String sourceSetName, Collection classpath) { + List types = getJavaStandardLibraryTypes(); + Map> gavToTypes = new LinkedHashMap<>(); + Map typeToGav = new LinkedHashMap<>(); + for (Path path : classpath) { + List typesFromPath = typesFromPath(path, null); + + types.addAll(typesFromPath); + String gav = gavFromPath(path); + if(gav != null) { + gavToTypes.put(gav, typesFromPath); + for(JavaType.FullyQualified type : typesFromPath) { + typeToGav.put(type, gav); + } + } + } + return new JavaSourceSet(randomId(), sourceSetName, types, gavToTypes, typeToGav); + } + + /** + * Assuming the provided path is to a jar file in a local maven repository or Gradle cache, derive the GAV coordinate from it. + * If no GAV can be determined returns null. + * + */ + @Nullable + static String gavFromPath(Path path) { + String pathStr = PathUtils.separatorsToUnix(path.toString()); + List pathParts = Arrays.asList(pathStr.split("/")); + // Example maven path: ~/.m2/repository/org/openrewrite/rewrite-core/8.32.0/rewrite-core-8.32.0.jar + // Example gradle path: ~/.gradle/caches/modules-2/files-2.1/org.openrewrite/rewrite-core/8.32.0/64ddcc371f1bf29593b4b27e907757d5554d1a83/rewrite-core-8.32.0.jar + + // Either of these directories may be relocated, so a fixed index is unreliable + String groupId = null; + String artifactId = null; + String version = null; + if (pathStr.contains("modules-2/files-2.1") && pathParts.size() >= 5) { + groupId = pathParts.get(pathParts.size() - 5); + artifactId = pathParts.get(pathParts.size() - 4); + version = pathParts.get(pathParts.size() - 3); + } else if (pathParts.size() >= 4) { + version = pathParts.get(pathParts.size() - 2); + artifactId = pathParts.get(pathParts.size() - 3); + // Unknown how many of the next several path parts will together comprise the groupId + // Maven repository root will have a "repository.xml" file + StringBuilder groupIdBuilder = new StringBuilder(); + int i = pathParts.size() - 3; + while(i > 0) { + Path maybeRepositoryRoot = Paths.get(String.join("/", pathParts.subList(0, i))); + if(maybeRepositoryRoot.endsWith("repository") || Files.exists(maybeRepositoryRoot.resolve("repository.xml"))) { + groupId = groupIdBuilder.substring(1); // Trim off the preceding "." + break; + } + groupIdBuilder.insert(0, "." + pathParts.get(i - 1)); + i--; + } + } + if(groupId == null || artifactId == null || version == null) { + return null; + } + return groupId + ":" + artifactId + ":" + version; + } + + + // Worth caching as there is typically substantial overlap in dependencies in use within the same repository + // Even a single module project will typically have at least two source sets, main and test + private static List typesFromPath(Path path, @Nullable String acceptPackage) { + List types = new ArrayList<>(); + try { + // Paths will be to either directories of class files or jar files + if (Files.isRegularFile(path)) { + try (JarFile jarFile = new JarFile(path.toFile())) { + Enumeration entries = jarFile.entries(); + while(entries.hasMoreElements()) { + String entryName = entries.nextElement().getName(); + if(entryName.endsWith(".class")) { + String s = entryNameToClassName(entryName); + if(isDeclarable(s)) { + types.add(JavaType.ShallowClass.build(s)); + } + } + } + } + } else { + Files.walkFileTree(path, new SimpleFileVisitor() { + @Override + public java.nio.file.FileVisitResult visitFile(Path file, java.nio.file.attribute.BasicFileAttributes attrs) { + String pathStr = file.toString(); + if (pathStr.endsWith(".class")) { + String s = entryNameToClassName(pathStr); + if((acceptPackage == null || s.startsWith(acceptPackage)) &&isDeclarable(s)) { + types.add(JavaType.ShallowClass.build(s)); + } + } + return java.nio.file.FileVisitResult.CONTINUE; + } + }); + } + } catch (IOException e) { + // Partial results better than no results + } + return types; + } + + private static List getJavaStandardLibraryTypes() { + List javaStandardLibraryTypes = new ArrayList<>(); + Path toolsJar = Paths.get(System.getProperty("java.home")).resolve("../lib/tools.jar"); + if(Files.exists(toolsJar)) { + javaStandardLibraryTypes.addAll(typesFromPath(toolsJar, "java")); + } else { + javaStandardLibraryTypes.addAll(typesFromPath( + FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules", "java.base"), + "java")); + } + return javaStandardLibraryTypes; + } + + private static String entryNameToClassName(String entryName) { + String result = entryName; + if(entryName.startsWith("modules/java.base/")) { + result = entryName.substring("modules/java.base/".length()); + } + return result.substring(0, result.length() - ".class".length()) + .replace('/', '.'); + } + + private static boolean isDeclarable(String className) { + int dotIndex = Math.max(className.lastIndexOf("."), className.lastIndexOf('$')); + className = className.substring(dotIndex + 1); char firstChar = className.charAt(0); - return !Character.isJavaIdentifierPart(firstChar) || Character.isDigit(firstChar); + return Character.isJavaIdentifierPart(firstChar) && !Character.isDigit(firstChar); } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 9712f7662de..49af5568e86 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -48,18 +48,12 @@ if(!file("IDE.properties").exists() || includedProjects.contains("tools")) { include(*allProjects.toTypedArray()) -allProjects.minus(includedProjects).forEach { - // sinkhole this project to a directory that intentionally doesn't exist, so that it - // can be efficiently substituted for a module dependency below - project(":$it").projectDir = file("sinkhole-$it") -} - gradle.allprojects { configurations.all { resolutionStrategy.dependencySubstitution { allProjects .minus(includedProjects) - .minus(arrayOf("rewrite-benchmarks", "rewrite-bom")) + .minus(arrayOf("rewrite-bom")) .forEach { substitute(project(":$it")) .using(module("org.openrewrite:$it:latest.integration")) @@ -79,7 +73,7 @@ if (System.getProperty("idea.active") == null && } // --------------------------------------------------------------- -// ------ Gradle Enterprise Configuration ------------------------ +// ------ Gradle Develocity Configuration ------------------------ // --------------------------------------------------------------- plugins {