Skip to content

Commit

Permalink
Remove ClassGraph from computation of JavaSourceSet, include type to …
Browse files Browse the repository at this point in the history 
…GAV coordinate mappings (#4391)

* 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 <[email protected]>
  • Loading branch information
@sambsnyd @jkschneider
sambsnyd and jkschneider authored Aug 7, 2024
1 parent 299a1fd commit 7d71f62
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 90 deletions.
38 changes: 38 additions & 0 deletions rewrite-benchmarks/src/jmh/java/org/openrewrite/benchmarks/java/JavaSourceSetBenchmark.java
View file
Original file line number Diff line number Diff line change
@@ -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<Path> 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);
}
}
94 changes: 26 additions & 68 deletions rewrite-java-test/src/test/java/org/openrewrite/java/marker/JavaSourceSetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<JavaType> 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<JavaType> uniqueTypes, JavaType root, boolean report) {
var newUnique = new AtomicBoolean(false);

new JavaTypeVisitor<Integer>() {
@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");
}
}
2 changes: 1 addition & 1 deletion rewrite-java/src/main/java/org/openrewrite/java/Assertions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}
Loading

0 comments on commit 7d71f62

Please sign in to comment.