Skip to content

Commit

Permalink
Huge speedup by separating "find" from "generate" (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
nedtwigg authored Jan 27, 2025
2 parents d691de0 + 69f9e10 commit 4c8d0a0
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 93 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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", "")
Expand All @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -73,7 +72,7 @@ class PlugParser {
}

companion object {
fun asmToJava(className: String): String {
private fun asmToJava(className: String): String {
return className.replace("/", ".")
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 ->
Expand Down
Loading

0 comments on commit 4c8d0a0

Please sign in to comment.