Improve performance of Java agent

scavenger-agent-java/build.gradle.kts

@@ -1,24 +1,28 @@
-import com.github.jengelman.gradle.plugins.shadow.tasks.ConfigureShadowRelocation
 import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
 
 plugins {
     java
     `maven-publish`
     signing
-    id("com.github.johnrengelman.shadow") version "8.0.0"
+    id("com.gradleup.shadow") version "8.3.3"
     id("io.freefair.lombok") version "8.6"
     id("org.unbroken-dome.test-sets") version "4.1.0"
 }
 
 java {
     toolchain {
-        languageVersion.set(JavaLanguageVersion.of(8))
+        languageVersion.set(JavaLanguageVersion.of(21))
     }
-
     withJavadocJar()
     withSourcesJar()
 }
 
+tasks.withType<JavaCompile>().matching {
+    it.name in setOf("compileJava", "compileIntegrationTestJava")
+}.configureEach {
+    options.release = 8
+}
+
 tasks.withType<ShadowJar> {
     archiveFileName.set("${project.name}-${project.version}.jar")
 
@@ -27,7 +31,9 @@ tasks.withType<ShadowJar> {
         attributes["Implementation-Version"] = project.version
     }
 
-    dependsOn("relocateShadowJar")
+    isEnableRelocation = true
+    relocationPrefix = "sc"
+
     mergeServiceFiles()
 
     minimize {
@@ -38,11 +44,6 @@ tasks.withType<ShadowJar> {
     exclude("**/*.kotlin_*")
 }
 
-tasks.register<ConfigureShadowRelocation>("relocateShadowJar") {
-    target = tasks.shadowJar.get()
-    prefix = "sc"
-}
-
 tasks.assemble {
     dependsOn(tasks.shadowJar)
 }
@@ -71,9 +72,11 @@ dependencies {
     implementation("io.grpc:grpc-okhttp:${property("grpcVersion")}")
 
     testImplementation(platform("org.junit:junit-bom:5.8.2"))
+    testImplementation(platform("org.mockito:mockito-bom:5.13.0"))
     testImplementation("org.junit.jupiter:junit-jupiter")
     testImplementation("org.assertj:assertj-core:3.22.0")
-    testImplementation("org.mockito:mockito-inline:4.3.1")
+    testImplementation("org.mockito:mockito-core")
+    testImplementation("org.mockito:mockito-junit-jupiter")
 }
 
 testSets {
@@ -85,6 +88,10 @@ dependencies {
     "integrationTestImplementation"("org.springframework.boot:spring-boot-starter-aop:2.5.12")
     "integrationTestImplementation"("com.github.tomakehurst:wiremock:2.27.2")
     "integrationTestImplementation"("org.grpcmock:grpcmock-junit5:0.13.0")
+
+    // JMH benchmarks
+    "integrationTestImplementation"("org.openjdk.jmh:jmh-core:1.37")
+    "integrationTestAnnotationProcessor"("org.openjdk.jmh:jmh-generator-annprocess:1.37")
 }
 
 fun javaPaths(vararg versions: Int) = versions.joinToString(",",

scavenger-agent-java/src/integrationTest/java/integrationTest/javaagent/AbstractWireMockTest.java

@@ -0,0 +1,37 @@
+package integrationTest.javaagent;
+
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import io.grpc.ManagedChannel;
+import io.grpc.ManagedChannelBuilder;
+import org.grpcmock.GrpcMock;
+import org.grpcmock.junit5.GrpcMockExtension;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import java.util.Optional;
+
+@ExtendWith(GrpcMockExtension.class)
+public class AbstractWireMockTest {
+
+    protected static WireMockServer wireMockServer;
+    protected static ManagedChannel channel;
+
+    @BeforeAll
+    static void startWireMockServer() {
+        wireMockServer = new WireMockServer(new WireMockConfiguration().dynamicPort());
+        wireMockServer.start();
+        WireMock.configureFor(wireMockServer.port());
+        channel = ManagedChannelBuilder.forAddress("localhost", GrpcMock.getGlobalPort())
+                .usePlaintext()
+                .build();
+    }
+
+    @AfterAll
+    static void shutDownWireMockServer() {
+        Optional.ofNullable(wireMockServer).ifPresent(WireMockServer::shutdown);
+        Optional.ofNullable(channel).ifPresent(ManagedChannel::shutdownNow);
+    }
+}

scavenger-agent-java/src/integrationTest/java/integrationTest/javaagent/BenchmarkTest.java

@@ -0,0 +1,28 @@
+package integrationTest.javaagent;
+
+import integrationTest.support.AgentBenchmarkExtension;
+import integrationTest.support.AgentRunner;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@Disabled("Comment/remove this annotation to run benchmarks")
+@ExtendWith(AgentBenchmarkExtension.class)
+public class BenchmarkTest {
+
+    @TestTemplate
+    @DisplayName("JMH benchmark")
+    void bench(AgentRunner agentRunner) throws Exception {
+        agentRunner.setShouldLogOutput(true);
+        agentRunner.call();
+    }
+
+    @TestTemplate
+    @DisplayName("JMH benchmark no advice")
+    void benchNoAdvice(AgentRunner agentRunner) throws Exception {
+        agentRunner.setConfigProperty("packages", "none");
+        agentRunner.setShouldLogOutput(true);
+        agentRunner.call();
+    }
+}

scavenger-agent-java/src/integrationTest/java/integrationTest/javaagent/ConfigTest.java

@@ -20,7 +20,7 @@ public class ConfigTest {
     @DisplayName("if no configuration is found")
     void noConfig(AgentRunner agentRunner) throws Exception {
         // given
-        agentRunner.setConfigFilePath("");
+        agentRunner.setConfigFilePath(null);
 
         // when
         String actual = agentRunner.call();
@@ -50,9 +50,7 @@ void nonExistentConfig(AgentRunner agentRunner) throws Exception {
     @DisplayName("if required field is not set")
     void missingRequiredTest(AgentRunner agentRunner) throws Exception {
         // given
-        Properties properties = new Properties();
-        properties.setProperty("packages", "");
-        agentRunner.setConfig(properties);
+        agentRunner.setConfigProperty("packages", "");
 
         // when
         String actual = agentRunner.call();

scavenger-agent-java/src/integrationTest/java/integrationTest/javaagent/InvocationTest.java

@@ -47,28 +47,8 @@
 import com.navercorp.scavenger.model.PublicationResponse;
 
 @ExtendWith(AgentIntegrationTestContextProvider.class)
-@ExtendWith(GrpcMockExtension.class)
 @DisplayName("invocation track test")
-public class InvocationTest {
-    private static WireMockServer wireMockServer;
-    private static ManagedChannel channel;
-
-    @BeforeAll
-    static void setUp() {
-        wireMockServer = new WireMockServer(new WireMockConfiguration().dynamicPort());
-        wireMockServer.start();
-        WireMock.configureFor(wireMockServer.port());
-
-        channel = ManagedChannelBuilder.forAddress("localhost", GrpcMock.getGlobalPort())
-            .usePlaintext()
-            .build();
-    }
-
-    @AfterAll
-    static void tearDown() {
-        Optional.ofNullable(wireMockServer).ifPresent(WireMockServer::shutdown);
-        Optional.ofNullable(channel).ifPresent(ManagedChannel::shutdownNow);
-    }
+public class InvocationTest extends AbstractWireMockTest {
 
     @TestTemplate
     @DisplayName("it tracks correctly")
@@ -88,10 +68,8 @@ void track(AgentRunner agentRunner) throws Exception {
     @DisplayName("it sends publication correctly")
     void send(AgentRunner agentRunner) throws Exception {
         // given
-        Properties properties = new Properties();
-        properties.setProperty("serverUrl", "http://localhost:" + wireMockServer.port());
-        properties.setProperty("schedulerInitialDelayMillis", "0");
-        agentRunner.setConfig(properties);
+        agentRunner.setConfigProperty("serverUrl", "http://localhost:" + wireMockServer.port());
+        agentRunner.setConfigProperty("schedulerInitialDelayMillis", "0");
 
         givenThat(
             get(V5_INIT_CONFIG + "?licenseKey=")

scavenger-agent-java/src/integrationTest/java/integrationTest/javaagent/ScanTest.java

@@ -5,6 +5,7 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.okJson;
 import static com.navercorp.scavenger.model.Endpoints.Agent.V5_INIT_CONFIG;
 import static integrationTest.util.AgentLogAssertionUtil.assertSampleAppOutput;
+import static integrationTest.util.AgentLogAssertionUtil.extractFromMatchingLogLines;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.grpcmock.GrpcMock.calledMethod;
 import static org.grpcmock.GrpcMock.getGlobalPort;
@@ -13,62 +14,33 @@
 import static org.grpcmock.GrpcMock.verifyThat;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.lang.reflect.Method;
-import java.util.Optional;
-import java.util.Properties;
+import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.grpcmock.GrpcMock;
-import org.grpcmock.junit5.GrpcMockExtension;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
+import com.navercorp.scavenger.javaagent.collecting.CodeBaseScanner;
+import com.navercorp.scavenger.javaagent.collecting.InvocationTracker;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.ExtendWith;
 
-import com.github.tomakehurst.wiremock.WireMockServer;
-import com.github.tomakehurst.wiremock.client.WireMock;
-import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import com.google.protobuf.util.JsonFormat;
 import integrationTest.support.AgentIntegrationTestContextProvider;
 import integrationTest.support.AgentRunner;
-import integrationTest.util.AgentLogAssertionUtil;
-import io.grpc.ManagedChannel;
-import io.grpc.ManagedChannelBuilder;
 import sample.app.NotServiceClass;
+import sample.app.SampleApp;
+import sample.app.SampleAspect;
 import sample.app.SampleService1;
-import sample.app.excluded.NotTrackedClass;
-import sample.app.excluded.NotTrackedClass2;
+import sample.app.SampleService2;
 
 import com.navercorp.scavenger.model.GetConfigResponse;
 import com.navercorp.scavenger.model.GrpcAgentServiceGrpc;
 import com.navercorp.scavenger.model.InitConfigResponse;
 import com.navercorp.scavenger.model.PublicationResponse;
 
 @ExtendWith(AgentIntegrationTestContextProvider.class)
-@ExtendWith(GrpcMockExtension.class)
 @DisplayName("codebase scan test")
-public class ScanTest {
-    private static WireMockServer wireMockServer;
-    private static ManagedChannel channel;
-
-    @BeforeAll
-    static void setUp() {
-        wireMockServer = new WireMockServer(new WireMockConfiguration().dynamicPort());
-        wireMockServer.start();
-        WireMock.configureFor(wireMockServer.port());
-
-        channel = ManagedChannelBuilder.forAddress("localhost", GrpcMock.getGlobalPort())
-            .usePlaintext()
-            .build();
-    }
-
-    @AfterAll
-    static void tearDown() {
-        Optional.ofNullable(wireMockServer).ifPresent(WireMockServer::shutdown);
-        Optional.ofNullable(channel).ifPresent(ManagedChannel::shutdownNow);
-    }
+public class ScanTest extends AbstractWireMockTest {
 
     @TestTemplate
     @DisplayName("it scans correctly")
@@ -78,20 +50,47 @@ void scan(AgentRunner agentRunner) throws Exception {
 
         // then
         assertSampleAppOutput(stdout);
-        assertThat(stdout).matches(scanned(SampleService1.class.getMethod("doSomething", int.class)));
-        assertThat(stdout).matches(scanned(NotServiceClass.class.getMethod("doSomething", int.class)));
-        assertThat(stdout).doesNotMatch(scanned(NotTrackedClass.class.getMethod("doSomething")));
-        assertThat(stdout).doesNotMatch(scanned(NotTrackedClass2.class.getMethod("doSomething")));
+        List<String> scannedMethods = extractFromMatchingLogLines(stdout, CodeBaseScanner.class, "[scavenger] ", " is scanned");
+        assertThat(scannedMethods).containsExactlyInAnyOrder(
+                "sample.app.NotServiceClass()",
+                "sample.app.NotServiceClass.doNothing()",
+                "sample.app.NotServiceClass.doSomething(int)",
+                "sample.app.SampleApp(sample.app.SampleService1)",
+                "sample.app.SampleApp.add(int,int)",
+                "sample.app.SampleApp.main(java.lang.String[])",
+                "sample.app.SampleApp.postConstruct()",
+                "sample.app.SampleAspect()",
+                "sample.app.SampleAspect.aroundSampleService(org.aspectj.lang.ProceedingJoinPoint)",
+                "sample.app.SampleAspect.logAspectLoaded()",
+                "sample.app.SampleService1(sample.app.SampleService2)",
+                "sample.app.SampleService1.doSomething(int)",
+                "sample.app.SampleService2()",
+                "sample.app.SampleService2.doSomething(int)"
+        );
+    }
+
+    @TestTemplate
+    @DisplayName("it installs advice correctly")
+    void advice(AgentRunner agentRunner) throws Exception {
+        String stdout = agentRunner.call();
+
+        List<String> installedAdvice = extractFromMatchingLogLines(stdout, InvocationTracker.class,
+                "[scavenger] Advice on ", " is installed");
+
+        assertThat(installedAdvice).containsExactlyInAnyOrder(
+                SampleApp.class.getName(),
+                SampleService1.class.getName(),
+                SampleAspect.class.getName(),
+                SampleService2.class.getName(),
+                NotServiceClass.class.getName());
     }
 
     @TestTemplate
     @DisplayName("it sends publication correctly")
     void send(AgentRunner agentRunner) throws Exception {
         // given
-        Properties properties = new Properties();
-        properties.setProperty("schedulerInitialDelayMillis", "0");
-        properties.setProperty("serverUrl", "http://localhost:" + wireMockServer.port());
-        agentRunner.setConfig(properties);
+        agentRunner.setConfigProperty("serverUrl", "http://localhost:" + wireMockServer.port());
+        agentRunner.setConfigProperty("schedulerInitialDelayMillis", "0");
 
         givenThat(
             get(V5_INIT_CONFIG + "?licenseKey=")
@@ -128,13 +127,6 @@ void send(AgentRunner agentRunner) throws Exception {
                 .withRequest(pub -> pub.getEntryCount() == getMethodsCount(stdout)));
     }
 
-    private static Pattern scanned(Method method) {
-        String[] split = method.toString().split(" ");
-        String signature = split[split.length - 1];
-        return AgentLogAssertionUtil.logPattern("com.navercorp.scavenger.javaagent.collecting.CodeBaseScanner",
-            "[scavenger] " + signature + " is scanned");
-    }
-
     private static int getMethodsCount(String stdout) {
         Matcher matcher = Pattern.compile("\\[scavenger] codebase\\(.*\\) scanned in \\d* ms: (\\d*) methods").matcher(stdout);
         assertTrue(matcher.find());

scavenger-agent-java/src/integrationTest/java/integrationTest/support/AgentBenchmarkExtension.java

@@ -0,0 +1,14 @@
+package integrationTest.support;
+
+public class AgentBenchmarkExtension extends AgentIntegrationTestContextProvider {
+
+    @Override
+    protected Class<?> getTestAppMainClass() {
+        return org.openjdk.jmh.Main.class;
+    }
+
+    @Override
+    protected String getScavengerConfigPath() {
+        return "scavenger-jmh.conf";
+    }
+}