diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/OrderedParallelWriter.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/OrderedParallelWriter.kt new file mode 100644 index 000000000..afa1e2881 --- /dev/null +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/OrderedParallelWriter.kt @@ -0,0 +1,44 @@ +package com.github.jengelman.gradle.plugins.shadow.internal + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking + +/** + * Computes entry bytes in parallel on [Dispatchers.Default] and writes them in submission order, + * which keeps the output deterministic. + * + * Entries written directly bypass the queue, so [flush] must be called before writing them. + */ +internal class OrderedParallelWriter(private val scope: CoroutineScope) { + private val pendingEntries = ArrayDeque() + + fun submit(compute: () -> ByteArray, write: (ByteArray) -> Unit) { + pendingEntries.addLast(PendingEntry(scope.async(Dispatchers.Default) { compute() }, write)) + if (pendingEntries.size >= MAX_PENDING_ENTRIES) { + pendingEntries.removeFirst().writeBytes() + } + } + + fun flush() { + while (pendingEntries.isNotEmpty()) { + pendingEntries.removeFirst().writeBytes() + } + } + + private fun PendingEntry.writeBytes() { + write(runBlocking { deferredBytes.await() }) + } + + private class PendingEntry(val deferredBytes: Deferred, val write: (ByteArray) -> Unit) + + private companion object { + /** + * Maximum number of in-flight parallel tasks in the sliding window. Bounds memory consumption + * on large archives while keeping worker threads saturated. + */ + const val MAX_PENDING_ENTRIES = 128 + } +} diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJar.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJar.kt index 595cd23eb..5d0b69b9d 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJar.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJar.kt @@ -4,6 +4,7 @@ import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator import com.github.jengelman.gradle.plugins.shadow.relocation.relocatePath import java.io.File import java.io.InputStream +import kotlinx.coroutines.runBlocking import org.apache.tools.zip.ZipOutputStream import org.gradle.api.file.FileCollection import org.vafer.jdeb.shaded.objectweb.asm.ClassReader @@ -19,7 +20,6 @@ internal fun generateSourcesJar( dependencies: Iterable, relocators: Iterable, unusedClasses: Set, - preserveFileTimestamps: Boolean, ) = try { zipOutStream.use { zos -> @@ -35,11 +35,7 @@ internal fun generateSourcesJar( val manifestEntry = "META-INF/MANIFEST.MF" visitedFiles.add(manifestEntry) - zos.writeEntry( - name = manifestEntry, - preserveLastModified = preserveFileTimestamps, - unixMode = UnixMode.file(), - ) { + zos.writeEntry(name = manifestEntry, unixMode = UnixMode.file()) { write("Manifest-Version: 1.0\n\n".toByteArray()) } @@ -50,70 +46,68 @@ internal fun generateSourcesJar( } } - // Relocates and writes a project or dependency source file, or a resource. - fun addEntry(name: String, lastModified: Long, readBytes: () -> ByteArray) { - if (name.isModuleInfo) return - val isSource = name.isSourceFile() - if (isSource && isUnused(name, unusedClasses, sourceToClasses)) return - val relocatedPath = - if (isSource) relocators.relocateSourcePath(name) else relocators.relocatePath(name) - if (!visitedFiles.add(relocatedPath)) return - val bytes = - if (isSource) { - relocators - .remapSourceFile(readBytes().decodeToString(), name, relocatedPath) - .toByteArray() - } else { - readBytes() - } - zos.writeEntry( - name = relocatedPath, - preserveLastModified = preserveFileTimestamps, - lastModified = lastModified, - unixMode = UnixMode.file(), - ) { - write(bytes) - } - } + runBlocking { + val parallelWriter = OrderedParallelWriter(this) - filesWithRelPaths - .sortedBy { it.second } - .forEach { (file, relPath) -> - addEntry(relPath, file.lastModified()) { file.readBytes() } + fun writeToZip(entryName: String, bytes: ByteArray) { + zos.writeEntry(name = entryName, unixMode = UnixMode.file()) { write(bytes) } } - sourcesJars.forEach { jarFile -> - jarFile.useZip { - entries() - .toList() - .filterNot { entry -> - val name = entry.name - entry.isDirectory || - name == "META-INF/MANIFEST.MF" || - name.endsWith(".class") || - name.startsWith("META-INF/INDEX.LIST") || - (name.startsWith("META-INF/") && - (name.endsWith(".SF") || name.endsWith(".DSA") || name.endsWith(".RSA"))) - } - .sortedBy { it.name } - .forEach { entry -> - addEntry(entry.name, entry.time) { getInputStream(entry).use { it.readBytes() } } - } + // Relocates and writes a project or dependency source file, or a resource. + fun addEntry(name: String, readBytes: () -> ByteArray) { + if (name.isModuleInfo) return + val isSource = name.isSourceFile() + if (isSource && isUnused(name, unusedClasses, sourceToClasses)) return + val relocatedPath = + if (isSource) relocators.relocateSourcePath(name) else relocators.relocatePath(name) + if (!visitedFiles.add(relocatedPath)) return + if (!isSource) { + // Resources gain nothing from parallelism, write them directly to avoid buffering. + parallelWriter.flush() + writeToZip(relocatedPath, readBytes()) + return + } + val rawBytes = readBytes() + parallelWriter.submit( + compute = { + relocators + .remapSourceFile(rawBytes.decodeToString(), name, relocatedPath) + .toByteArray() + }, + write = { writeToZip(relocatedPath, it) }, + ) } - } - val entries = zos.entries.map { it.name } - val added = entries.toMutableSet() - entries.forEach { name -> - name.parentDirectoryEntries().forEach { entryName -> - if (!added.add(entryName)) return@forEach - zos.writeEntry( - name = entryName, - preserveLastModified = preserveFileTimestamps, - unixMode = UnixMode.directory(), - ) + filesWithRelPaths + .sortedBy { it.second } + .forEach { (file, relPath) -> + addEntry(relPath) { file.readBytes() } + } + + sourcesJars.forEach { jarFile -> + jarFile.useZip { + entries() + .toList() + .filterNot { entry -> + val name = entry.name + entry.isDirectory || + name == "META-INF/MANIFEST.MF" || + name.endsWith(".class") || + name.startsWith("META-INF/INDEX.LIST") || + (name.startsWith("META-INF/") && + (name.endsWith(".SF") || name.endsWith(".DSA") || name.endsWith(".RSA"))) + } + .sortedBy { it.name } + .forEach { entry -> + addEntry(entry.name) { getInputStream(entry).use { it.readBytes() } } + } + } } + + parallelWriter.flush() } + + zos.writeParentDirectoryEntries() } } catch (e: Exception) { sourcesJarFile.delete() diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/Zip.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/Zip.kt index a9f483e17..ba2b8f6a2 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/Zip.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/Zip.kt @@ -8,6 +8,7 @@ import org.apache.tools.zip.UnixStat import org.apache.tools.zip.Zip64Mode import org.apache.tools.zip.ZipEntry import org.apache.tools.zip.ZipOutputStream +import org.gradle.api.file.FileCopyDetails import org.gradle.api.tasks.bundling.ZipEntryCompression @JvmInline @@ -103,6 +104,38 @@ internal inline fun ZipOutputStream.writeEntry( } } +/** + * Writes the missing parent directory entries of all written entries, taking the timestamp and + * permissions from [visitedDirs] when available. + */ +internal fun ZipOutputStream.writeParentDirectoryEntries( + preserveLastModified: Boolean = false, + visitedDirs: Map = emptyMap(), +) { + val entries = entries.map { it.name } + val added = entries.toMutableSet() + val currentTimeMillis = System.currentTimeMillis() + + entries.forEach { name -> + name.parentDirectoryEntries().asReversed().forEach { entryName -> + if (!added.add(entryName)) return@forEach + val details = visitedDirs[entryName.removeSuffix("/")] + val (lastModified, unixMode) = + if (details == null) { + currentTimeMillis to UnixMode.directory() + } else { + details.lastModified to UnixMode.directory(details.permissions.toUnixNumeric()) + } + writeEntry( + name = entryName, + preserveLastModified = preserveLastModified, + lastModified = lastModified, + unixMode = unixMode, + ) + } + } +} + internal fun String.parentDirectoryEntries(): List { val parents = mutableListOf() var parent = substringBeforeLast('/', "") diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.kt index cc966785a..dd3b30b15 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/relocation/Relocator.kt @@ -9,7 +9,7 @@ import org.gradle.api.tasks.Input * [org.apache.maven.plugins.shade.relocation.Relocator.java](https://github.com/apache/maven-shade-plugin/blob/master/src/main/java/org/apache/maven/plugins/shade/relocation/Relocator.java). * * Implementations of [Relocator] must be thread-safe, as their methods may be invoked concurrently - * across multiple worker threads during parallel bytecode remapping. + * across multiple worker threads during parallel bytecode and source remapping. * * @author Jason van Zyl * @author John Engelman 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 4fdc22a1d..360cf2497 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 @@ -4,23 +4,20 @@ package com.github.jengelman.gradle.plugins.shadow.tasks +import com.github.jengelman.gradle.plugins.shadow.internal.OrderedParallelWriter import com.github.jengelman.gradle.plugins.shadow.internal.UnixMode 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.internal.writeParentDirectoryEntries import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator 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 kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.async import kotlinx.coroutines.runBlocking import org.apache.tools.zip.Zip64RequiredException import org.apache.tools.zip.ZipOutputStream @@ -80,13 +77,13 @@ internal constructor( try { zipOutStream.use { zos -> runBlocking { - val action = StreamAction(zos, this) + val action = StreamAction(zos, OrderedParallelWriter(this)) stream.process(action) action.flush() } processTransformers(zos) - addDirs(zos) + zos.writeParentDirectoryEntries(isPreserveFileTimestamps, visitedDirs) checkDuplicateEntries(zos) } } catch (e: Exception) { @@ -121,31 +118,6 @@ internal constructor( } } - private fun addDirs(zos: ZipOutputStream) { - val entries = zos.entries.map { it.name } - val added = entries.toMutableSet() - val currentTimeMillis = System.currentTimeMillis() - - entries.forEach { name -> - name.parentDirectoryEntries().asReversed().forEach { entryName -> - if (!added.add(entryName)) return@forEach - val details = visitedDirs[entryName.removeSuffix("/")] - val (lastModified, unixMode) = - if (details == null) { - currentTimeMillis to UnixMode.directory() - } else { - details.lastModified to UnixMode.directory(details.permissions.toUnixNumeric()) - } - zos.writeEntry( - name = entryName, - preserveLastModified = isPreserveFileTimestamps, - lastModified = lastModified, - unixMode = unixMode, - ) - } - } - } - private fun checkDuplicateEntries(zos: ZipOutputStream) { val entries = zos.entries.map { it.name } val duplicates = entries.groupingBy { it }.eachCount().filter { it.value > 1 } @@ -163,10 +135,8 @@ internal constructor( private inner class StreamAction( private val zipOutStr: ZipOutputStream, - private val scope: CoroutineScope, + private val parallelWriter: OrderedParallelWriter, ) : CopyActionProcessingStreamAction { - private val pendingEntries = ArrayDeque() - init { logger.info("Relocator count: {}.", relocators.size) } @@ -196,38 +166,22 @@ internal constructor( val pathSuffix = path.removePrefix(multiReleasePrefix) val relocatedPath = multiReleasePrefix + relocators.relocatePath(pathSuffix) val rawBytes = fileDetails.readBytes() - val deferred = - scope.async(Dispatchers.Default) { - rawBytes.remapClass(relocators = relocators, path = path) - } - pendingEntries.addLast( - PendingEntry( - entryName = relocatedPath, - fileDetails = fileDetails, - deferredBytes = deferred, - ), + parallelWriter.submit( + compute = { rawBytes.remapClass(relocators = relocators, path = path) }, + write = { fileDetails.writeToZip(relocatedPath, it) }, ) - if (pendingEntries.size >= MAX_PENDING_ENTRIES) { - runBlocking { pendingEntries.removeFirst().writeToZip() } - } } } else -> { val relocated = relocators.relocatePath(path) if (transform(fileDetails, relocated)) return - if (pendingEntries.isNotEmpty()) { - runBlocking { flush() } - } + parallelWriter.flush() fileDetails.writeToZip(relocated) } } } - suspend fun flush() { - while (pendingEntries.isNotEmpty()) { - pendingEntries.removeFirst().writeToZip() - } - } + fun flush() = parallelWriter.flush() private fun isUnused(classPath: String): Boolean { val className = classPath.substringBeforeLast(".").replace('/', '.') @@ -249,10 +203,6 @@ internal constructor( return true } - private suspend fun PendingEntry.writeToZip() { - fileDetails.writeToZip(entryName, deferredBytes.await()) - } - private fun FileCopyDetails.writeToZip(entryName: String, bytes: ByteArray? = null) { zipOutStr.writeEntry( name = entryName, @@ -269,22 +219,10 @@ internal constructor( } } - private class PendingEntry( - val entryName: String, - val fileDetails: FileCopyDetails, - val deferredBytes: Deferred, - ) - public companion object { private val logger = Logging.getLogger(@Suppress("DEPRECATION") ShadowCopyAction::class.java) private val multiReleaseRegex = "^META-INF/versions/\\d+/".toRegex() - /** - * Maximum number of in-flight parallel class remapping tasks in the sliding window. Bounds - * memory consumption on large archives while keeping worker threads saturated. - */ - private const val MAX_PENDING_ENTRIES = 128 - @Deprecated( message = "Use `ShadowJar.CONSTANT_TIME_FOR_ZIP_ENTRIES` constant instead. This will be removed in Shadow 10.", diff --git a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt index 7f36db284..506766240 100644 --- a/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt +++ b/src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.kt @@ -210,6 +210,9 @@ public abstract class ShadowJar : Jar() { * `java.withSourcesJar()` is enabled, and `false` otherwise. In projects applying the `shadow` * plugin for Kotlin Multiplatform, this convention defaults to `true` when sources publishing is * enabled on the JVM target, and `false` otherwise. + * + * Entries in the shadowed sources JAR always use [CONSTANT_TIME_FOR_ZIP_ENTRIES] as their + * timestamp, regardless of [isPreserveFileTimestamps]. */ @get:Input @get:Option( @@ -846,7 +849,6 @@ public abstract class ShadowJar : Jar() { dependencies = includedDependencies.files, relocators = relocators.get() + packageRelocators, unusedClasses = unusedClasses, - preserveFileTimestamps = isPreserveFileTimestamps, ) } diff --git a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJarTest.kt b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJarTest.kt index 5b4cb5011..1521f3fcf 100644 --- a/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJarTest.kt +++ b/src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ShadowedSourcesJarTest.kt @@ -120,8 +120,8 @@ class ShadowedSourcesJarTest { "META-INF/", "a/", "m/", - "z/", "z/sub/", + "z/", ) } } @@ -269,15 +269,53 @@ class ShadowedSourcesJarTest { "shaded/dep/Dep.java", "shaded/dep/resource.txt", "META-INF/", - "com/", "com/example/", - "shaded/", + "com/", "shaded/dep/", + "shaded/", ) getContent("shaded/dep/Dep.java").isEqualTo("package shaded.dep;\npublic class Dep {}") getContent("shaded/dep/resource.txt").isEqualTo("dep resource") } } + + @Test + fun remapsManySourcesInParallel() { + val srcDir = tempDir.resolve("src").createDirectories() + val names = (1..500).map { "Class%03d".format(it) } + names.forEach { name -> + srcDir.resolve("com/example/$name.java").apply { + createParentDirectories() + writeText("package com.example;\npublic class $name {}") + } + srcDir.resolve("com/example/$name.txt").writeText("resource $name") + } + + val outputJar = tempDir.resolve("output-sources.jar") + generateSourcesJar( + sourcesJarFile = outputJar.toFile(), + sourceSetsSourceDirs = testObjectFactory.fileCollection().from(srcDir), + relocators = listOf(SimpleRelocator("com.example", "shaded.example")), + ) + + val expectedEntries = names.flatMap { + listOf("shaded/example/$it.java", "shaded/example/$it.txt") + } + assertThat(JarPath(outputJar)).useAll { + containsExactly( + "META-INF/MANIFEST.MF", + *expectedEntries.toTypedArray(), + "META-INF/", + "shaded/example/", + "shaded/", + ) + names.forEach { name -> + getContent("shaded/example/$name.java") + .isEqualTo("package shaded.example;\npublic class $name {}") + getContent("shaded/example/$name.txt").isEqualTo("resource $name") + } + } + } } private fun generateSourcesJar( @@ -301,5 +339,4 @@ private fun generateSourcesJar( dependencies = emptyList(), relocators = relocators, unusedClasses = emptySet(), - preserveFileTimestamps = true, )