From 220a8753579f4a1d7b972b80da55240aa0f79a58 Mon Sep 17 00:00:00 2001 From: Goooler Date: Sat, 12 Sep 2026 22:51:06 +0800 Subject: [PATCH 1/8] Use ExecutorService for parallel bytecode remapping --- .../shadow/internal/RelocatorRemapper.kt | 42 +++--- .../plugins/shadow/tasks/ShadowCopyAction.kt | 122 ++++++++++++++---- 2 files changed, 116 insertions(+), 48 deletions(-) diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt index 0c33dbb28..14d144f13 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt @@ -13,28 +13,28 @@ import org.vafer.jdeb.shaded.objectweb.asm.commons.Remapper * (possibly) remapped class bytes. If no remapping is required, the original bytes are returned. */ internal fun FileCopyDetails.remapClass(relocators: Set): ByteArray = - inputStream() - .use { it.readBytes() } - .let { bytes -> - var modified = false - val remapper = RelocatorRemapper(relocators) { modified = true } + remapClass(bytes = inputStream().use { it.readBytes() }, path = path, relocators = relocators) - // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant - // pool. Copying the original constant pool should be avoided because it would keep references - // to the original class names. This is not a problem at runtime (because these entries in the - // constant pool are never used), but confuses some tools such as Felix's maven-bundle-plugin - // that use the constant pool to determine the dependencies of a class. - try { - val cw = ClassWriter(0) - val cr = ClassReader(bytes) - val cv = ClassRemapper(cw, remapper) - cr.accept(cv, ClassReader.EXPAND_FRAMES) - // If we didn't need to change anything, keep the original bytes as-is. - if (modified) cw.toByteArray() else bytes - } catch (t: Throwable) { - gradleError("Error in ASM processing class $path", t) - } - } +internal fun remapClass(bytes: ByteArray, path: String, relocators: Set): ByteArray { + var modified = false + val remapper = RelocatorRemapper(relocators) { modified = true } + + // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant + // pool. Copying the original constant pool should be avoided because it would keep references + // to the original class names. This is not a problem at runtime (because these entries in the + // constant pool are never used), but confuses some tools such as Felix's maven-bundle-plugin + // that use the constant pool to determine the dependencies of a class. + return try { + val cw = ClassWriter(0) + val cr = ClassReader(bytes) + val cv = ClassRemapper(cw, remapper) + cr.accept(cv, ClassReader.EXPAND_FRAMES) + // If we didn't need to change anything, keep the original bytes as-is. + if (modified) cw.toByteArray() else bytes + } catch (t: Throwable) { + gradleError("Error in ASM processing class $path", t) + } +} private class RelocatorRemapper( private val relocators: Set, diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt index 305287327..a08ec67f9 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt @@ -16,6 +16,11 @@ import com.github.jengelman.gradle.plugins.shadow.relocation.relocatePath import com.github.jengelman.gradle.plugins.shadow.transformers.ResourceTransformer import com.github.jengelman.gradle.plugins.shadow.transformers.TransformerContext import java.io.File +import java.util.concurrent.ArrayBlockingQueue +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import org.apache.tools.zip.Zip64RequiredException import org.apache.tools.zip.ZipOutputStream import org.gradle.api.file.FileCopyDetails @@ -71,9 +76,36 @@ internal constructor( private val visitedDirs = mutableMapOf() override fun execute(stream: CopyActionProcessingStream): WorkResult { + val threadPool: ExecutorService = + Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors().coerceAtLeast(2)) + val queue = ArrayBlockingQueue(128) + try { zipOutStream.use { zos -> - stream.process(StreamAction(zos)) + val writer = threadPool.submit { + while (true) { + val item = queue.take() + if (item === POISON_PILL) break + val bytes = item.futureBytes.get() + zos.writeEntry( + name = item.entryName, + preserveLastModified = isPreserveFileTimestamps, + lastModified = item.lastModified, + unixMode = item.unixMode, + ) { + write(bytes) + } + } + } + + try { + stream.process(StreamAction(threadPool, queue)) + } finally { + queue.put(POISON_PILL) + } + + writer.get() + processTransformers(zos) addDirs(zos) checkDuplicateEntries(zos) @@ -98,6 +130,9 @@ internal constructor( } zipFile.delete() throw e + } finally { + threadPool.shutdown() + threadPool.awaitTermination(1, TimeUnit.MINUTES) } return WorkResults.didWork(true) } @@ -150,8 +185,17 @@ internal constructor( } } - private inner class StreamAction(private val zipOutStr: ZipOutputStream) : - CopyActionProcessingStreamAction { + private class ProcessItem( + val entryName: String, + val futureBytes: CompletableFuture, + val lastModified: Long, + val unixMode: UnixMode, + ) + + private inner class StreamAction( + private val executor: ExecutorService, + private val queue: ArrayBlockingQueue, + ) : CopyActionProcessingStreamAction { init { logger.info("Relocator count: {}.", relocators.size) } @@ -173,26 +217,58 @@ internal constructor( when { path.endsWith(".class") -> { if (isUnused(path)) return + val rawBytes = fileDetails.inputStream().use { it.readBytes() } if (relocators.isEmpty()) { - fileDetails.writeToZip(path) + sendEntry( + entryName = path, + fileDetails = fileDetails, + futureBytes = CompletableFuture.completedFuture(rawBytes), + ) } else { - with(fileDetails) { - // Temporarily remove the multi-release prefix. - val multiReleasePrefix = multiReleaseRegex.find(path)?.value.orEmpty() - val pathSuffix = path.removePrefix(multiReleasePrefix) - val relocatedPath = multiReleasePrefix + relocators.relocatePath(pathSuffix) - writeToZip(entryName = relocatedPath, bytes = remapClass(relocators = relocators)) - } + // Temporarily remove the multi-release prefix. + val multiReleasePrefix = multiReleaseRegex.find(path)?.value.orEmpty() + val pathSuffix = path.removePrefix(multiReleasePrefix) + val relocatedPath = multiReleasePrefix + relocators.relocatePath(pathSuffix) + val future = + CompletableFuture.supplyAsync( + { remapClass(bytes = rawBytes, path = path, relocators = relocators) }, + executor, + ) + sendEntry( + entryName = relocatedPath, + fileDetails = fileDetails, + futureBytes = future, + ) } } else -> { val relocated = relocators.relocatePath(path) if (transform(fileDetails, relocated)) return - fileDetails.writeToZip(relocated) + val rawBytes = fileDetails.inputStream().use { it.readBytes() } + sendEntry( + entryName = relocated, + fileDetails = fileDetails, + futureBytes = CompletableFuture.completedFuture(rawBytes), + ) } } } + private fun sendEntry( + entryName: String, + fileDetails: FileCopyDetails, + futureBytes: CompletableFuture, + ) { + queue.put( + ProcessItem( + entryName = entryName, + futureBytes = futureBytes, + lastModified = fileDetails.lastModified, + unixMode = UnixMode.file(fileDetails.permissions.toUnixNumeric()), + ) + ) + } + private fun isUnused(classPath: String): Boolean { val className = classPath.substringBeforeLast(".").replace('/', '.') return unusedClasses.contains(className).also { @@ -212,26 +288,18 @@ internal constructor( } return true } - - private fun FileCopyDetails.writeToZip(entryName: String, bytes: ByteArray? = null) { - zipOutStr.writeEntry( - name = entryName, - preserveLastModified = isPreserveFileTimestamps, - lastModified = lastModified, - unixMode = UnixMode.file(permissions.toUnixNumeric()), - ) { - if (bytes == null) { - copyTo(this) - } else { - write(bytes) - } - } - } } public companion object { private val logger = Logging.getLogger(@Suppress("DEPRECATION") ShadowCopyAction::class.java) private val multiReleaseRegex = "^META-INF/versions/\\d+/".toRegex() + private val POISON_PILL = + ProcessItem( + entryName = "", + futureBytes = CompletableFuture.completedFuture(ByteArray(0)), + lastModified = 0L, + unixMode = UnixMode.file(), + ) @Deprecated( message = From c842256c8564d407e35b3ca7dec3c7791f92650d Mon Sep 17 00:00:00 2001 From: Goooler Date: Sat, 12 Sep 2026 22:56:50 +0800 Subject: [PATCH 2/8] Use Kotlin Coroutines for parallel bytecode remapping --- build.gradle.kts | 1 + gradle/libs.versions.toml | 1 + .../plugins/shadow/tasks/ShadowCopyAction.kt | 107 ++++++++---------- 3 files changed, 52 insertions(+), 57 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index ae2dcc51a..3341108e9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -131,6 +131,7 @@ dependencies { compileOnly(libs.develocity) compileOnly(libs.kotlin.gradlePlugin) compileOnly(libs.kotlin.reflect) + compileOnly(libs.kotlinx.coroutines) api(libs.apache.ant) // Types from Ant are exposed in the public API. implementation(libs.apache.log4j) implementation(libs.jdependency) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index d66e43746..3e9da1ee8 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -17,6 +17,7 @@ plexus-xml = "org.codehaus.plexus:plexus-xml:4.2.0" xmlunit = "org.xmlunit:xmlunit-legacy:2.13.0" moshi = { module = "com.squareup.moshi:moshi", version.ref = "moshi" } moshi-kotlin = { module = "com.squareup.moshi:moshi-kotlin", version.ref = "moshi" } +kotlinx-coroutines = "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.1" foojayResolver = "org.gradle.toolchains.foojay-resolver-convention:org.gradle.toolchains.foojay-resolver-convention.gradle.plugin:1.0.0" develocity = "com.gradle:develocity-gradle-plugin:4.5.1" diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt index a08ec67f9..54503fb4e 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt @@ -16,11 +16,14 @@ import com.github.jengelman.gradle.plugins.shadow.relocation.relocatePath import com.github.jengelman.gradle.plugins.shadow.transformers.ResourceTransformer import com.github.jengelman.gradle.plugins.shadow.transformers.TransformerContext import java.io.File -import java.util.concurrent.ArrayBlockingQueue -import java.util.concurrent.CompletableFuture -import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.apache.tools.zip.Zip64RequiredException import org.apache.tools.zip.ZipOutputStream import org.gradle.api.file.FileCopyDetails @@ -76,36 +79,35 @@ internal constructor( private val visitedDirs = mutableMapOf() override fun execute(stream: CopyActionProcessingStream): WorkResult { - val threadPool: ExecutorService = - Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors().coerceAtLeast(2)) - val queue = ArrayBlockingQueue(128) - try { zipOutStream.use { zos -> - val writer = threadPool.submit { - while (true) { - val item = queue.take() - if (item === POISON_PILL) break - val bytes = item.futureBytes.get() - zos.writeEntry( - name = item.entryName, - preserveLastModified = isPreserveFileTimestamps, - lastModified = item.lastModified, - unixMode = item.unixMode, - ) { - write(bytes) + runBlocking { + val channel = Channel(capacity = 128) + + val writer = + launch(Dispatchers.Default) { + for (item in channel) { + val bytes = item.deferredBytes.await() + zos.writeEntry( + name = item.entryName, + preserveLastModified = isPreserveFileTimestamps, + lastModified = item.lastModified, + unixMode = item.unixMode, + ) { + write(bytes) + } + } } + + try { + stream.process(StreamAction(this, channel)) + } finally { + channel.close() } - } - try { - stream.process(StreamAction(threadPool, queue)) - } finally { - queue.put(POISON_PILL) + writer.join() } - writer.get() - processTransformers(zos) addDirs(zos) checkDuplicateEntries(zos) @@ -130,9 +132,6 @@ internal constructor( } zipFile.delete() throw e - } finally { - threadPool.shutdown() - threadPool.awaitTermination(1, TimeUnit.MINUTES) } return WorkResults.didWork(true) } @@ -187,14 +186,14 @@ internal constructor( private class ProcessItem( val entryName: String, - val futureBytes: CompletableFuture, + val deferredBytes: Deferred, val lastModified: Long, val unixMode: UnixMode, ) private inner class StreamAction( - private val executor: ExecutorService, - private val queue: ArrayBlockingQueue, + private val scope: CoroutineScope, + private val channel: Channel, ) : CopyActionProcessingStreamAction { init { logger.info("Relocator count: {}.", relocators.size) @@ -222,22 +221,21 @@ internal constructor( sendEntry( entryName = path, fileDetails = fileDetails, - futureBytes = CompletableFuture.completedFuture(rawBytes), + deferredBytes = CompletableDeferred(rawBytes), ) } else { // Temporarily remove the multi-release prefix. val multiReleasePrefix = multiReleaseRegex.find(path)?.value.orEmpty() val pathSuffix = path.removePrefix(multiReleasePrefix) val relocatedPath = multiReleasePrefix + relocators.relocatePath(pathSuffix) - val future = - CompletableFuture.supplyAsync( - { remapClass(bytes = rawBytes, path = path, relocators = relocators) }, - executor, - ) + val deferred = + scope.async(Dispatchers.Default) { + remapClass(bytes = rawBytes, path = path, relocators = relocators) + } sendEntry( entryName = relocatedPath, fileDetails = fileDetails, - futureBytes = future, + deferredBytes = deferred, ) } } @@ -248,7 +246,7 @@ internal constructor( sendEntry( entryName = relocated, fileDetails = fileDetails, - futureBytes = CompletableFuture.completedFuture(rawBytes), + deferredBytes = CompletableDeferred(rawBytes), ) } } @@ -257,16 +255,18 @@ internal constructor( private fun sendEntry( entryName: String, fileDetails: FileCopyDetails, - futureBytes: CompletableFuture, + deferredBytes: Deferred, ) { - queue.put( - ProcessItem( - entryName = entryName, - futureBytes = futureBytes, - lastModified = fileDetails.lastModified, - unixMode = UnixMode.file(fileDetails.permissions.toUnixNumeric()), + runBlocking { + channel.send( + ProcessItem( + entryName = entryName, + deferredBytes = deferredBytes, + lastModified = fileDetails.lastModified, + unixMode = UnixMode.file(fileDetails.permissions.toUnixNumeric()), + ) ) - ) + } } private fun isUnused(classPath: String): Boolean { @@ -293,13 +293,6 @@ internal constructor( public companion object { private val logger = Logging.getLogger(@Suppress("DEPRECATION") ShadowCopyAction::class.java) private val multiReleaseRegex = "^META-INF/versions/\\d+/".toRegex() - private val POISON_PILL = - ProcessItem( - entryName = "", - futureBytes = CompletableFuture.completedFuture(ByteArray(0)), - lastModified = 0L, - unixMode = UnixMode.file(), - ) @Deprecated( message = From af14270efcc70e545a3c187e17200c35136ac189 Mon Sep 17 00:00:00 2001 From: Goooler Date: Sat, 12 Sep 2026 23:17:36 +0800 Subject: [PATCH 3/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfc196d0d..c8827b984 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - `XmlAppendingTransformer` - Append terminating newline in `ServiceFileTransformer`. ([#2202](https://github.com/GradleUp/shadow/pull/2202)) - Remove redundant JAR normalization for R8 output. ([#2236](https://github.com/GradleUp/shadow/pull/2236)) +- Parallelize bytecode remapping in `ShadowCopyAction`. ([#2302](https://github.com/GradleUp/shadow/pull/2302)) ### Deprecated From 40d172308ec96e0b0852f49cbe1087403607202e Mon Sep 17 00:00:00 2001 From: Goooler Date: Sat, 12 Sep 2026 23:29:17 +0800 Subject: [PATCH 4/8] Add ParallelRelocationTest --- .../plugins/shadow/ParallelRelocationTest.kt | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/ParallelRelocationTest.kt diff --git a/src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/ParallelRelocationTest.kt b/src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/ParallelRelocationTest.kt new file mode 100644 index 000000000..84d948ce7 --- /dev/null +++ b/src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/ParallelRelocationTest.kt @@ -0,0 +1,117 @@ +package com.github.jengelman.gradle.plugins.shadow + +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.isEqualTo +import com.github.jengelman.gradle.plugins.shadow.testkit.classLoader +import com.github.jengelman.gradle.plugins.shadow.testkit.containsExactly +import com.github.jengelman.gradle.plugins.shadow.testkit.loadClass +import kotlin.io.path.appendText +import kotlin.io.path.readBytes +import org.junit.jupiter.api.Test + +class ParallelRelocationTest : BasePluginTest() { + @Test + fun largeNumberOfClassesWithRelocation() { + val count = 500 + val classNames = (1..count).map { "Class%03d".format(it) } + val largeJar = + buildJar("many-classes.jar") { + for (name in classNames) { + insert( + "com/example/pkg/$name.class", + createEmptyClassBytes("com/example/pkg/$name"), + ) + } + } + + projectScript.appendText( + """ + |dependencies { + | ${implementationFiles(largeJar)} + |} + |$shadowJarTask { + | relocate 'com.example.pkg', 'relocated.pkg' + |} + """ + .trimMargin() + ) + + runWithSuccess(shadowJarPath) + + assertThat(outputShadowedJar).useAll { + val relocatedEntries = classNames.map { "relocated/pkg/$it.class" }.toTypedArray() + containsExactly( + "META-INF/MANIFEST.MF", + *relocatedEntries, + "META-INF/", + "relocated/pkg/", + "relocated/", + ) + classLoader { + loadClass("relocated.pkg.${classNames.first()}") + loadClass("relocated.pkg.${classNames.last()}") + } + } + } + + @Test + fun deterministicZipEntryOrderAcrossMultipleBuilds() { + val count = 150 + val testJar = + buildJar("deterministic-test.jar") { + for (i in 1..count) { + insert( + "com/example/test/TestClass$i.class", + createEmptyClassBytes("com/example/test/TestClass$i"), + ) + insert("resources/res_$i.txt", "content $i") + } + } + + projectScript.appendText( + """ + |dependencies { + | ${implementationFiles(testJar)} + |} + |$shadowJarTask { + | relocate 'com.example.test', 'shadowed.example.test' + |} + """ + .trimMargin() + ) + + runWithSuccess(shadowJarPath) + val firstBytes = path("build/libs/my-1.0-all.jar").readBytes() + + runWithSuccess(shadowJarPath, "--rerun-tasks") + val secondBytes = path("build/libs/my-1.0-all.jar").readBytes() + + assertThat(firstBytes).isEqualTo(secondBytes) + } + + @Test + fun errorPropagationWhenClassIsCorrupted() { + val badClassEntry = "corrupt/BadClass.class" + val corruptJar = + buildJar("corrupt.jar") { + insert(badClassEntry, byteArrayOf(0xCA.toByte(), 0xFE.toByte(), 0xBA.toByte())) + } + + projectScript.appendText( + """ + |dependencies { + | ${implementationFiles(corruptJar)} + |} + |$shadowJarTask { + | relocate 'corrupt', 'relocated.corrupt' + |} + """ + .trimMargin() + ) + + val result = runWithFailure(shadowJarPath) + + assertThat(result.output).contains("Error in ASM processing class $badClassEntry") + } +} From bf3d49134ad19f35c8771288fc8fff0c6ac0f6e3 Mon Sep 17 00:00:00 2001 From: Goooler Date: Sat, 12 Sep 2026 23:55:54 +0800 Subject: [PATCH 5/8] Add parallel vs sequential remapping performance test --- build.gradle.kts | 5 +- .../shadow/internal/BytecodeRemappingTest.kt | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 3341108e9..50bf1a68d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -153,7 +153,10 @@ dependencies { testing.suites { named("test") { - dependencies { implementation(libs.xmlunit) } + dependencies { + implementation(libs.kotlinx.coroutines) + implementation(libs.xmlunit) + } } register("documentTest") { targets.configureEach { diff --git a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt index 027866b1f..a9a21c462 100644 --- a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt +++ b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt @@ -6,6 +6,7 @@ import assertk.assertions.containsExactly import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf +import assertk.assertions.isLessThan import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator import com.github.jengelman.gradle.plugins.shadow.testkit.requireResourceAsPath import com.github.jengelman.gradle.plugins.shadow.util.noOpDelegate @@ -16,11 +17,18 @@ import kotlin.io.path.copyTo import kotlin.io.path.createParentDirectories import kotlin.io.path.inputStream import kotlin.io.path.invariantSeparatorsPathString +import kotlin.io.path.readBytes import kotlin.io.path.relativeTo import kotlin.io.path.writeBytes import kotlin.io.path.writeText import kotlin.metadata.jvm.KotlinClassMetadata import kotlin.reflect.KClass +import kotlin.time.Duration +import kotlin.time.measureTime +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.runBlocking import org.gradle.api.GradleException import org.gradle.api.file.FileCopyDetails import org.junit.jupiter.api.Test @@ -335,6 +343,45 @@ class BytecodeRemappingTest { .containsExactly("kotlin/jvm/internal/Intrinsics", relocatedFixtureBase) } + @Test + fun parallelRemappingFasterThanSequential() { + val rawBytes = + requireResourceAsPath("${FixtureSubject::class.java.name.replace('.', '/')}.class") + .readBytes() + val classes = (1..1000).map { "com/example/Class$it.class" to rawBytes } + + fun remapSequential() = classes.map { (path, bytes) -> + remapClass(bytes = bytes, path = path, relocators = relocators) + } + + fun remapParallel() = runBlocking { + classes + .map { (path, bytes) -> + async(Dispatchers.Default) { + remapClass(bytes = bytes, path = path, relocators = relocators) + } + } + .awaitAll() + } + + // Warm up JIT and coroutines thread pool + repeat(3) { + remapSequential() + remapParallel() + } + + var sequentialDuration = Duration.ZERO + var parallelDuration = Duration.ZERO + val iterations = 5 + repeat(iterations) { + sequentialDuration += measureTime { remapSequential() } + parallelDuration += measureTime { remapParallel() } + } + + val ratio = if (Runtime.getRuntime().availableProcessors() == 1) 1.0 else 0.8 + assertThat(parallelDuration).isLessThan(sequentialDuration * ratio) + } + private fun Path.toFileCopyDetails() = object : FileCopyDetails by noOpDelegate() { From 75db6201cc1baeceb070093f5cb3ddad6a94fb8d Mon Sep 17 00:00:00 2001 From: Goooler Date: Sun, 13 Sep 2026 12:27:30 +0800 Subject: [PATCH 6/8] Revert "Add parallel vs sequential remapping performance test" This reverts commit bf3d49134ad19f35c8771288fc8fff0c6ac0f6e3. --- build.gradle.kts | 5 +- .../shadow/internal/BytecodeRemappingTest.kt | 47 ------------------- 2 files changed, 1 insertion(+), 51 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 50bf1a68d..3341108e9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -153,10 +153,7 @@ dependencies { testing.suites { named("test") { - dependencies { - implementation(libs.kotlinx.coroutines) - implementation(libs.xmlunit) - } + dependencies { implementation(libs.xmlunit) } } register("documentTest") { targets.configureEach { diff --git a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt index a9a21c462..027866b1f 100644 --- a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt +++ b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/BytecodeRemappingTest.kt @@ -6,7 +6,6 @@ import assertk.assertions.containsExactly import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf -import assertk.assertions.isLessThan import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator import com.github.jengelman.gradle.plugins.shadow.testkit.requireResourceAsPath import com.github.jengelman.gradle.plugins.shadow.util.noOpDelegate @@ -17,18 +16,11 @@ import kotlin.io.path.copyTo import kotlin.io.path.createParentDirectories import kotlin.io.path.inputStream import kotlin.io.path.invariantSeparatorsPathString -import kotlin.io.path.readBytes import kotlin.io.path.relativeTo import kotlin.io.path.writeBytes import kotlin.io.path.writeText import kotlin.metadata.jvm.KotlinClassMetadata import kotlin.reflect.KClass -import kotlin.time.Duration -import kotlin.time.measureTime -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.async -import kotlinx.coroutines.awaitAll -import kotlinx.coroutines.runBlocking import org.gradle.api.GradleException import org.gradle.api.file.FileCopyDetails import org.junit.jupiter.api.Test @@ -343,45 +335,6 @@ class BytecodeRemappingTest { .containsExactly("kotlin/jvm/internal/Intrinsics", relocatedFixtureBase) } - @Test - fun parallelRemappingFasterThanSequential() { - val rawBytes = - requireResourceAsPath("${FixtureSubject::class.java.name.replace('.', '/')}.class") - .readBytes() - val classes = (1..1000).map { "com/example/Class$it.class" to rawBytes } - - fun remapSequential() = classes.map { (path, bytes) -> - remapClass(bytes = bytes, path = path, relocators = relocators) - } - - fun remapParallel() = runBlocking { - classes - .map { (path, bytes) -> - async(Dispatchers.Default) { - remapClass(bytes = bytes, path = path, relocators = relocators) - } - } - .awaitAll() - } - - // Warm up JIT and coroutines thread pool - repeat(3) { - remapSequential() - remapParallel() - } - - var sequentialDuration = Duration.ZERO - var parallelDuration = Duration.ZERO - val iterations = 5 - repeat(iterations) { - sequentialDuration += measureTime { remapSequential() } - parallelDuration += measureTime { remapParallel() } - } - - val ratio = if (Runtime.getRuntime().availableProcessors() == 1) 1.0 else 0.8 - assertThat(parallelDuration).isLessThan(sequentialDuration * ratio) - } - private fun Path.toFileCopyDetails() = object : FileCopyDetails by noOpDelegate() { From 7d8fe173a7d7506391fb226950b28ccd9eebc848 Mon Sep 17 00:00:00 2001 From: Goooler Date: Sun, 13 Sep 2026 12:31:07 +0800 Subject: [PATCH 7/8] Cleanups --- gradle/libs.versions.toml | 2 +- .../plugins/shadow/internal/GradleCompat.kt | 2 + .../shadow/internal/RelocatorRemapper.kt | 39 +++++++-------- .../plugins/shadow/tasks/ShadowCopyAction.kt | 47 +++++++++---------- 4 files changed, 44 insertions(+), 46 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 3e9da1ee8..ac1f16920 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -12,12 +12,12 @@ jdependency = "org.vafer:jdependency:2.16" jdom2 = "org.jdom:jdom2:2.0.6.1" kotlin-metadata = { module = "org.jetbrains.kotlin:kotlin-metadata-jvm", version.ref = "kotlin" } kotlin-reflect = { module = "org.jetbrains.kotlin:kotlin-reflect", version.ref = "kotlin" } +kotlinx-coroutines = "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.1" plexus-utils = "org.codehaus.plexus:plexus-utils:4.1.0" plexus-xml = "org.codehaus.plexus:plexus-xml:4.2.0" xmlunit = "org.xmlunit:xmlunit-legacy:2.13.0" moshi = { module = "com.squareup.moshi:moshi", version.ref = "moshi" } moshi-kotlin = { module = "com.squareup.moshi:moshi-kotlin", version.ref = "moshi" } -kotlinx-coroutines = "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.1" foojayResolver = "org.gradle.toolchains.foojay-resolver-convention:org.gradle.toolchains.foojay-resolver-convention.gradle.plugin:1.0.0" develocity = "com.gradle:develocity-gradle-plugin:4.5.1" diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/GradleCompat.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/GradleCompat.kt index a565aa0da..f6c317801 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/GradleCompat.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/GradleCompat.kt @@ -74,6 +74,8 @@ internal fun FileTreeElement.inputStream(): InputStream = file.inputStream() } +internal fun FileTreeElement.readBytes(): ByteArray = inputStream().use(InputStream::readBytes) + internal inline fun ObjectFactory.property( defaultValue: Any? = null ): Property = diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt index 14d144f13..2e93f03d4 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/RelocatorRemapper.kt @@ -13,28 +13,29 @@ import org.vafer.jdeb.shaded.objectweb.asm.commons.Remapper * (possibly) remapped class bytes. If no remapping is required, the original bytes are returned. */ internal fun FileCopyDetails.remapClass(relocators: Set): ByteArray = - remapClass(bytes = inputStream().use { it.readBytes() }, path = path, relocators = relocators) + readBytes().remapClass(relocators = relocators, path = path) -internal fun remapClass(bytes: ByteArray, path: String, relocators: Set): ByteArray { - var modified = false - val remapper = RelocatorRemapper(relocators) { modified = true } +internal fun ByteArray.remapClass(relocators: Set, path: String): ByteArray = + let { bytes -> + var modified = false + val remapper = RelocatorRemapper(relocators) { modified = true } - // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant - // pool. Copying the original constant pool should be avoided because it would keep references - // to the original class names. This is not a problem at runtime (because these entries in the - // constant pool are never used), but confuses some tools such as Felix's maven-bundle-plugin - // that use the constant pool to determine the dependencies of a class. - return try { - val cw = ClassWriter(0) - val cr = ClassReader(bytes) - val cv = ClassRemapper(cw, remapper) - cr.accept(cv, ClassReader.EXPAND_FRAMES) - // If we didn't need to change anything, keep the original bytes as-is. - if (modified) cw.toByteArray() else bytes - } catch (t: Throwable) { - gradleError("Error in ASM processing class $path", t) + // We don't pass the ClassReader here. This forces the ClassWriter to rebuild the constant + // pool. Copying the original constant pool should be avoided because it would keep references + // to the original class names. This is not a problem at runtime (because these entries in the + // constant pool are never used), but confuses some tools such as Felix's maven-bundle-plugin + // that use the constant pool to determine the dependencies of a class. + try { + val cw = ClassWriter(0) + val cr = ClassReader(bytes) + val cv = ClassRemapper(cw, remapper) + cr.accept(cv, ClassReader.EXPAND_FRAMES) + // If we didn't need to change anything, keep the original bytes as-is. + if (modified) cw.toByteArray() else bytes + } catch (t: Throwable) { + gradleError("Error in ASM processing class $path", t) + } } -} private class RelocatorRemapper( private val relocators: Set, diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt index 54503fb4e..279d5cf33 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowCopyAction.kt @@ -9,6 +9,7 @@ import com.github.jengelman.gradle.plugins.shadow.internal.entries import com.github.jengelman.gradle.plugins.shadow.internal.gradleError import com.github.jengelman.gradle.plugins.shadow.internal.inputStream import com.github.jengelman.gradle.plugins.shadow.internal.parentDirectoryEntries +import com.github.jengelman.gradle.plugins.shadow.internal.readBytes import com.github.jengelman.gradle.plugins.shadow.internal.remapClass import com.github.jengelman.gradle.plugins.shadow.internal.writeEntry import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator @@ -87,14 +88,13 @@ internal constructor( val writer = launch(Dispatchers.Default) { for (item in channel) { - val bytes = item.deferredBytes.await() zos.writeEntry( name = item.entryName, preserveLastModified = isPreserveFileTimestamps, lastModified = item.lastModified, unixMode = item.unixMode, ) { - write(bytes) + write(item.deferredBytes.await()) } } } @@ -184,13 +184,6 @@ internal constructor( } } - private class ProcessItem( - val entryName: String, - val deferredBytes: Deferred, - val lastModified: Long, - val unixMode: UnixMode, - ) - private inner class StreamAction( private val scope: CoroutineScope, private val channel: Channel, @@ -216,11 +209,10 @@ internal constructor( when { path.endsWith(".class") -> { if (isUnused(path)) return - val rawBytes = fileDetails.inputStream().use { it.readBytes() } + val rawBytes = fileDetails.readBytes() if (relocators.isEmpty()) { - sendEntry( + fileDetails.sendEntry( entryName = path, - fileDetails = fileDetails, deferredBytes = CompletableDeferred(rawBytes), ) } else { @@ -228,33 +220,29 @@ internal constructor( val multiReleasePrefix = multiReleaseRegex.find(path)?.value.orEmpty() val pathSuffix = path.removePrefix(multiReleasePrefix) val relocatedPath = multiReleasePrefix + relocators.relocatePath(pathSuffix) - val deferred = - scope.async(Dispatchers.Default) { - remapClass(bytes = rawBytes, path = path, relocators = relocators) - } - sendEntry( + fileDetails.sendEntry( entryName = relocatedPath, - fileDetails = fileDetails, - deferredBytes = deferred, + deferredBytes = + scope.async(Dispatchers.Default) { + rawBytes.remapClass(relocators = relocators, path = path) + }, ) } } else -> { val relocated = relocators.relocatePath(path) if (transform(fileDetails, relocated)) return - val rawBytes = fileDetails.inputStream().use { it.readBytes() } - sendEntry( + val rawBytes = fileDetails.readBytes() + fileDetails.sendEntry( entryName = relocated, - fileDetails = fileDetails, deferredBytes = CompletableDeferred(rawBytes), ) } } } - private fun sendEntry( + private fun FileCopyDetails.sendEntry( entryName: String, - fileDetails: FileCopyDetails, deferredBytes: Deferred, ) { runBlocking { @@ -262,8 +250,8 @@ internal constructor( ProcessItem( entryName = entryName, deferredBytes = deferredBytes, - lastModified = fileDetails.lastModified, - unixMode = UnixMode.file(fileDetails.permissions.toUnixNumeric()), + lastModified = lastModified, + unixMode = UnixMode.file(permissions.toUnixNumeric()), ) ) } @@ -290,6 +278,13 @@ internal constructor( } } + private class ProcessItem( + val entryName: String, + val deferredBytes: Deferred, + val lastModified: Long, + val unixMode: UnixMode, + ) + public companion object { private val logger = Logging.getLogger(@Suppress("DEPRECATION") ShadowCopyAction::class.java) private val multiReleaseRegex = "^META-INF/versions/\\d+/".toRegex() From e5fb6038dd8edd0aee220089fa81cb15f124634c Mon Sep 17 00:00:00 2001 From: Goooler Date: Sun, 13 Sep 2026 16:43:31 +0800 Subject: [PATCH 8/8] Apply coroutines explicitly for test sources --- build.gradle.kts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 3341108e9..50bf1a68d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -153,7 +153,10 @@ dependencies { testing.suites { named("test") { - dependencies { implementation(libs.xmlunit) } + dependencies { + implementation(libs.kotlinx.coroutines) + implementation(libs.xmlunit) + } } register("documentTest") { targets.configureEach {