Huge speedup by separating "find" from "generate"

CHANGELOG.md

@@ -1,6 +1,9 @@
 # AtPlug releases
 
 ## [Unreleased]
+### Added
+- Cacheability, incremental task, and huge speed improvement. ([#66](https://github.com/diffplug/atplug/pull/66))
+  - `plugGenerate` refactored into `plugFind` followed by `plugGenerate` 
 ### Changed
 - Bump required JVM from 11 to 17. ([#63](https://github.com/diffplug/atplug/pull/63))
 - Detect Kotlin version rather than harcode it. ([#64](https://github.com/diffplug/atplug/pull/64))

atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGenerator.kt

@@ -22,18 +22,18 @@ import java.lang.UnsatisfiedLinkError
 import java.lang.reflect.Constructor
 import java.lang.reflect.Modifier
 import java.net.URLClassLoader
-import java.nio.file.FileVisitResult
-import java.nio.file.Files
-import java.nio.file.Path
-import java.nio.file.SimpleFileVisitor
-import java.nio.file.attribute.BasicFileAttributes
 import java.util.*
 import java.util.function.Function
 import kotlin.reflect.KFunction
 import kotlin.reflect.full.companionObjectInstance
 import kotlin.reflect.full.memberFunctions
 
-class PlugGenerator internal constructor(toSearches: List<File>, toLinkAgainst: Set<File>) {
+class PlugGenerator
+internal constructor(
+		plugToSocket: Map<String, String>,
+		toSearches: List<File>,
+		toLinkAgainst: Set<File>
+) {
 	@JvmField val atplugInf: SortedMap<String, String> = TreeMap()
 
 	/** A cache from a plugin interface to a function that converts a class into its metadata. */
@@ -56,22 +56,7 @@ class PlugGenerator internal constructor(toSearches: List<File>, toLinkAgainst:
 				}!!
 						as KFunction<Function<Any, String>>
 		try {
-			val parser = PlugParser()
-			// walk toSearch, passing each classfile to load()
-			for (toSearch in toSearches) {
-				if (toSearch.isDirectory) {
-					Files.walkFileTree(
-							toSearch.toPath(),
-							object : SimpleFileVisitor<Path>() {
-								override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult {
-									if (file.toString().endsWith(EXT_CLASS)) {
-										maybeGeneratePlugin(parser, file)
-									}
-									return FileVisitResult.CONTINUE
-								}
-							})
-				}
-			}
+			plugToSocket.forEach { (plug, socket) -> atplugInf[plug] = generatePlugin(plug, socket) }
 		} finally {
 			classLoader.close()
 			System.setProperty("atplug.generate", "")
@@ -82,18 +67,13 @@ class PlugGenerator internal constructor(toSearches: List<File>, toLinkAgainst:
 	 * Loads a class by its FQN. If it's concrete and implements a plugin, then we'll call
 	 * generatePlugin.
 	 */
-	private fun maybeGeneratePlugin(parser: PlugParser, path: Path) {
-		parser.parse(path.toFile())
-		if (!parser.hasPlug()) {
-			return
-		}
-		val plugClass = classLoader.loadClass(parser.plugClassName)
-		val socketClass = classLoader.loadClass(parser.socketClassName)
+	private fun generatePlugin(plugClassName: String, socketClassName: String): String {
+		val plugClass = classLoader.loadClass(plugClassName)
+		val socketClass = classLoader.loadClass(socketClassName)
 		require(!Modifier.isAbstract(plugClass.modifiers)) {
 			"Class $plugClass has @Plug($socketClass) but it is abstract."
 		}
-		val atplugInfContent = generatePlugin<Any, Any>(plugClass, socketClass)
-		atplugInf[plugClass.name] = atplugInfContent
+		return generatePlugin<Any, Any>(plugClass, socketClass)
 	}
 
 	private fun <SocketT, PlugT : SocketT> generatePlugin(
@@ -128,14 +108,18 @@ class PlugGenerator internal constructor(toSearches: List<File>, toLinkAgainst:
 		 * Returns a Map from a plugin's name to its ATPLUG-INF content.
 		 *
 		 * @param toSearch a directory containing class files where we will look for plugin
-		 * implementations
+		 *   implementations
 		 * @param toLinkAgainst the classes that these plugins implementations need
 		 * @return a map from component name to is ATPLUG-INF string content
 		 */
-		fun generate(toSearch: List<File>, toLinkAgainst: Set<File>): SortedMap<String, String> {
+		fun generate(
+				plugToSocket: Map<String, String>,
+				toSearch: List<File>,
+				toLinkAgainst: Set<File>
+		): SortedMap<String, String> {
 			return try {
-				val ext = PlugGeneratorJavaExecable(toSearch, toLinkAgainst)
-				val metadataGen = PlugGenerator(ext.toSearch, ext.toLinkAgainst)
+				val ext = PlugGeneratorJavaExecable(plugToSocket, toSearch, toLinkAgainst)
+				val metadataGen = PlugGenerator(plugToSocket, ext.toSearch, ext.toLinkAgainst)
 				// save our results, with no reference to the guts of what happened inside PluginMetadataGen
 				metadataGen.atplugInf
 			} catch (e: Exception) {

atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGeneratorJavaExecable.kt

@@ -20,21 +20,21 @@ import java.io.File
 import java.util.*
 
 /** [PlugGenerator.PlugGenerator] in a [JavaExecable] form. */
-class PlugGeneratorJavaExecable(toSearch: List<File>?, toLinkAgainst: Set<File>?) : JavaExecable {
+class PlugGeneratorJavaExecable(
+		plugToSocket: Map<String, String>,
+		toSearch: List<File>,
+		toLinkAgainst: Set<File>
+) : JavaExecable {
 	// inputs
-	var toSearch: List<File>
-	var toLinkAgainst: Set<File>
+	var plugToSocket: Map<String, String> = LinkedHashMap(plugToSocket)
+	var toSearch: List<File> = ArrayList(toSearch)
+	var toLinkAgainst: Set<File> = LinkedHashSet(toLinkAgainst)
 
 	// outputs
 	@JvmField var atplugInf: SortedMap<String, String>? = null
 
-	init {
-		this.toSearch = ArrayList(toSearch)
-		this.toLinkAgainst = LinkedHashSet(toLinkAgainst)
-	}
-
 	override fun run() {
-		val metadataGen = PlugGenerator(toSearch, toLinkAgainst)
+		val metadataGen = PlugGenerator(plugToSocket, toSearch, toLinkAgainst)
 		atplugInf = metadataGen.atplugInf
 	}
 }

atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugParser.kt

@@ -26,7 +26,7 @@ import org.objectweb.asm.Type
 class PlugParser {
 	private var buffer = ByteArray(64 * 1024)
 
-	fun parse(file: File) {
+	fun parse(file: File): Pair<String, String>? {
 		val filelen = (file.length() + 1).toInt() // +1 prevents infinite loop
 		if (buffer.size < filelen) {
 			buffer = ByteArray(filelen)
@@ -42,18 +42,17 @@ class PlugParser {
 			}
 		}
 		val reader = ClassReader(buffer, 0, pos)
-		plugClassName = asmToJava(reader.className)
+		val plugClassName = asmToJava(reader.className)
+		this.plugClassName = plugClassName
 		socketClassName = null
+		// set socketClassName if there is an `@Plug`
 		reader.accept(
 				classVisitor, ClassReader.SKIP_FRAMES or ClassReader.SKIP_DEBUG or ClassReader.SKIP_CODE)
+		return socketClassName?.let { plugClassName to it }
 	}
 
-	fun hasPlug(): Boolean {
-		return socketClassName != null
-	}
-
-	var plugClassName: String? = null
-	var socketClassName: String? = null
+	private var plugClassName: String? = null
+	private var socketClassName: String? = null
 
 	private val classVisitor: ClassVisitor =
 			object : ClassVisitor(API) {
@@ -73,7 +72,7 @@ class PlugParser {
 			}
 
 	companion object {
-		fun asmToJava(className: String): String {
+		private fun asmToJava(className: String): String {
 			return className.replace("/", ".")
 		}
 

atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/FindPlugsTask.kt

@@ -0,0 +1,82 @@
+package com.diffplug.atplug.tooling.gradle
+
+import com.diffplug.atplug.tooling.PlugParser
+import java.io.File
+import org.gradle.api.DefaultTask
+import org.gradle.api.file.ConfigurableFileCollection
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.tasks.*
+import org.gradle.work.*
+
+/**
+ * Incrementally scans compiled classes for @Plug usage and writes discovered plug classes into an
+ * output directory.
+ */
+@CacheableTask
+abstract class FindPlugsTask : DefaultTask() {
+	@get:CompileClasspath
+	@get:Incremental
+	@get:InputFiles
+	abstract val classesFolders: ConfigurableFileCollection
+
+	/** Directory where we will store discovered plugs. */
+	@get:OutputDirectory abstract val discoveredPlugsDir: DirectoryProperty
+
+	@TaskAction
+	fun findPlugs(inputChanges: InputChanges) {
+		// If not incremental, clear everything and rescan
+		if (!inputChanges.isIncremental) {
+			discoveredPlugsDir.get().asFile.deleteRecursively()
+		}
+
+		// Make sure our output directory exists
+		discoveredPlugsDir.get().asFile.mkdirs()
+
+		// For each changed file in classesFolders, determine if it has @Plug
+		val parser = PlugParser()
+		for (change in inputChanges.getFileChanges(classesFolders)) {
+			if (!change.file.name.endsWith(".class")) {
+				continue
+			}
+			when (change.changeType) {
+				ChangeType.REMOVED -> {
+					// Remove old discovered data for this file
+					removeOldMetadata(change.file)
+				}
+				ChangeType.ADDED,
+				ChangeType.MODIFIED -> {
+					parseAndWriteMetadata(parser, change.file)
+				}
+			}
+		}
+	}
+
+	private fun parseAndWriteMetadata(parser: PlugParser, classFile: File) {
+		val plugToSocket = parser.parse(classFile)
+		if (plugToSocket != null) {
+			// For example: write a single line containing the discovered plug FQN
+			val discoveredFile = discoveredPlugsDir.file(classFile.nameWithoutExtension).get().asFile
+			if (discoveredFile.exists()) {
+				val existing = discoveredFile.readText().split("|")
+				check(existing[0] == plugToSocket.first) {
+					"You need to rename one of these plugs because they have the same classfile name: ${existing[0]} and $plugToSocket"
+				}
+			} else {
+				discoveredFile.parentFile.mkdirs()
+			}
+			discoveredFile.writeText(plugToSocket.let { "${it.first}|${it.second}" })
+		} else {
+			// If previously discovered, remove it
+			removeOldMetadata(classFile)
+		}
+	}
+
+	private fun removeOldMetadata(classFile: File) {
+		// Remove any discovered file for the old .class
+		val possibleName = classFile.nameWithoutExtension
+		val discoveredFile = discoveredPlugsDir.file(possibleName).get().asFile
+		if (discoveredFile.exists()) {
+			discoveredFile.delete()
+		}
+	}
+}

atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugGenerateTask.kt

@@ -30,20 +30,15 @@ import java.util.jar.Manifest
 import javax.inject.Inject
 import org.gradle.api.DefaultTask
 import org.gradle.api.file.ConfigurableFileCollection
+import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.FileCollection
 import org.gradle.api.plugins.JavaPluginExtension
 import org.gradle.api.provider.Property
-import org.gradle.api.tasks.Classpath
-import org.gradle.api.tasks.InputFiles
-import org.gradle.api.tasks.Internal
-import org.gradle.api.tasks.JavaExec
-import org.gradle.api.tasks.Nested
-import org.gradle.api.tasks.Optional
-import org.gradle.api.tasks.OutputDirectory
-import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.*
 import org.gradle.jvm.toolchain.JavaLauncher
 import org.gradle.jvm.toolchain.JavaToolchainService
 import org.gradle.process.JavaForkOptions
+import org.gradle.work.Incremental
 import org.gradle.workers.ProcessWorkerSpec
 import org.gradle.workers.WorkerExecutor
 
@@ -52,6 +47,8 @@ abstract class PlugGenerateTask : DefaultTask() {
 
 	@get:Inject abstract val workerExecutor: WorkerExecutor
 
+	@get:InputDirectory abstract val discoveredPlugsDir: DirectoryProperty
+
 	@get:InputFiles @get:Classpath abstract val jarsToLinkAgainst: ConfigurableFileCollection
 
 	@get:Internal var resourcesFolder: File? = null
@@ -60,7 +57,7 @@ abstract class PlugGenerateTask : DefaultTask() {
 	val atplugInfFolder: File
 		get() = File(resourcesFolder, PlugPlugin.ATPLUG_INF)
 
-	@InputFiles var classesFolders: FileCollection? = null
+	@get:Incremental @get:InputFiles abstract val classesFolders: ConfigurableFileCollection
 
 	init {
 		this.outputs.upToDateWhen {
@@ -74,20 +71,22 @@ abstract class PlugGenerateTask : DefaultTask() {
 		launcher.set(service.launcherFor(spec))
 	}
 
-	fun setClassesFolders(files: Iterable<File>) {
-		// if we don't copy, Gradle finds an implicit dependency which
-		// forces us to depend on `classes` even though we don't
-		val copy: MutableList<File> = ArrayList()
-		for (file in files) {
-			copy.add(file)
-		}
-		classesFolders = project.files(copy)
-	}
-
 	@TaskAction
 	fun build() {
+		val discoveredFiles = discoveredPlugsDir.get().asFile.listFiles().orEmpty().filter { it.isFile }
+		val plugsToSockets =
+				discoveredFiles.associate {
+					val pieces = it.readText().split("|")
+					pieces[0] to pieces[1]
+				}
+		if (plugsToSockets.isEmpty()) {
+			// no discovered plugs
+			FileMisc.cleanDir(atplugInfFolder)
+			return
+		}
+
 		// generate the metadata
-		val result = generate()
+		val result = generate(plugsToSockets)
 
 		// clean out the ATPLUG-INF folder, and put the map's content into the folder
 		FileMisc.cleanDir(atplugInfFolder)
@@ -97,8 +96,8 @@ abstract class PlugGenerateTask : DefaultTask() {
 		}
 
 		// the resources directory *needs* the Service-Component entry of the manifest to exist in order
-		// for tests to work
-		// so we'll get a manifest (empty if necessary, but preferably we'll load what already exists)
+		// for tests to work so we'll get a manifest (empty if necessary, but preferably we'll load what
+		// already exists)
 		val manifest = loadManifest()
 		val componentsCmd = atplugComponents(atplugInfFolder)
 		val componentsActual = manifest.mainAttributes.getValue(PlugPlugin.SERVICE_COMPONENT)
@@ -138,9 +137,10 @@ abstract class PlugGenerateTask : DefaultTask() {
 		}
 	}
 
-	private fun generate(): SortedMap<String, String> {
+	private fun generate(discoveredPlugs: Map<String, String>): SortedMap<String, String> {
 		val input =
-				PlugGeneratorJavaExecable(ArrayList(classesFolders!!.files), jarsToLinkAgainst.files)
+				PlugGeneratorJavaExecable(
+						discoveredPlugs, ArrayList(classesFolders.files), jarsToLinkAgainst.files)
 		return if (launcher.isPresent) {
 			val workQueue =
 					workerExecutor.processIsolation { workerSpec: ProcessWorkerSpec ->