Skip to content

Commit

Permalink
Avoid unnecessary Cargo crate rebuilds
Browse files Browse the repository at this point in the history
This commit modifies the Gradle buildscripts to avoid unnecessary Cargo
rebuilds of the generated crates, decreasing development iteration
cycles.

Prior to this commit, if you ran a crate-generating command _twice_ on
any of the `codegen-test`, `codegen-server-test`, `sdk-codegen-test`,
and `codegen-server-test:python` subprojects (without making any changes
to the codegen code), both invocations would take the same time: Cargo
would recompile the crate and its dependencies, even when the generated
crate is identical. This is because Gradle deletes everything under
`buildDir` when running the `generateSmithyBuild` task:

```
> Task :codegen-server-test:smithyBuildJar
Deleting stale output file: /local/home/davidpz/workplace/smithy-ws/src/SmithyRsSource/codegen-server-test/build/smithyprojections/codegen-server-test
```

So the files get recreated each time.

While developing, it is likely that only a small number of the generated
crate files are modified across rebuilds. [Cargo uses
`mtime`](rust-lang/cargo#6529) (among other
factors) to determine whether it needs to recompile a unit. Indeed,
running with `CARGO_LOG=cargo::core::compiler::fingerprint=trace` yields
`err: current filesystem status shows we're outdated`. This commit adds
a Gradle task that compares the hashes of the newly generated files with
the (previously cached) old ones, and restores their `mtime`s if the
hashes coincide.

Another issue was causing unnecessary crate rebuilds. Prior to this
commit, we were sending `RUSTFLAGS=-D warnings` when invoking Cargo.
However, a common thing to do after generating a crate is to open its
contents in an editor. The editor's `rust-analyzer` would compile the
crate and its dependencies without the `RUSTFLAGS` we had used earlier.
The next time you rebuilt the crate, Cargo would claim `err: RUSTFLAGS
has changed: previously [], now ["-D", "warnings"]` and recompile
everything again. This commit refactors the Gradle tasks so as to not
send these flags when invoking Cargo, instead generating a
`.cargo/config.toml` containing these flags. This way, `rust-analyzer`
also picks them up and does not need to recompile the crates.

With both patches, Cargo avoids unnecessary crate rebuilds. All in all,
the second invocation of a `./gradlew --info -P modules='simple' -P
cargoCommands='test' codegen-server-test:build` command now takes 15
seconds less than the first invocation on my `c5.4xlarge` machine; Cargo
does not need to do _any_ work on the second invocation.

This commit also refactors the `build.gradle.kts` files of the `sdk`,
`sdk-codegen-test`, `codegen-test`, `codegen-server-test`, and
`codegen-server-test:python` subprojects to make them DRYer and more
consistent. The last 4 subprojects' buildscripts are now much shorter,
with all the common logic having been moved to `CodegenTestCommon.kt`.
Note that we have made the last 4 subprojects' `cargo check` and `cargo
doc` invocations use the same set of flags than in the `sdk` subproject
for consistency.

Closes #1378.
Closes #1412.
  • Loading branch information
david-perez committed May 30, 2022
1 parent c503deb commit 668fbb4
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 273 deletions.
98 changes: 12 additions & 86 deletions aws/sdk-codegen-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ extra["moduleName"] = "software.amazon.smithy.kotlin.codegen.test"

tasks["jar"].enabled = false

plugins {
id("software.amazon.smithy").version("0.5.3")
}
plugins { id("software.amazon.smithy").version("0.5.3") }

val smithyVersion: String by project
val defaultRustFlags: String by project
val defaultRustDocFlags: String by project
val properties = PropertyRetriever(rootProject, project)

val pluginName = "rust-codegen"
val workingDirUnderBuildDir = "smithyprojections/sdk-codegen-test/"

dependencies {
implementation(project(":aws:sdk-codegen"))
Expand All @@ -23,94 +24,19 @@ dependencies {
implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
}

data class CodegenTest(val service: String, val module: String, val extraConfig: String? = null)

val CodegenTests = listOf(
val allCodegenTests = listOf(
CodegenTest("com.amazonaws.apigateway#BackplaneControlService", "apigateway")
)

fun generateSmithyBuild(tests: List<CodegenTest>): String {
val projections = tests.joinToString(",\n") {
"""
"${it.module}": {
"plugins": {
"rust-codegen": {
"runtimeConfig": {
"relativePath": "${rootProject.projectDir.absolutePath}/rust-runtime"
},
"service": "${it.service}",
"module": "${it.module}",
"moduleAuthors": ["[email protected]"],
"moduleVersion": "0.0.1"
${it.extraConfig ?: ""}
}
}
}
""".trimIndent()
}
return """
{
"version": "1.0",
"projections": { $projections }
}
"""
}

task("generateSmithyBuild") {
description = "generate smithy-build.json"
doFirst {
buildDir.resolve("smithy-build.json").writeText(generateSmithyBuild(CodegenTests))
}
}

fun generateCargoWorkspace(tests: List<CodegenTest>): String {
return """
[workspace]
members = [
${tests.joinToString(",") { "\"${it.module}/rust-codegen\"" }}
]
""".trimIndent()
}
task("generateCargoWorkspace") {
description = "generate Cargo.toml workspace file"
doFirst {
buildDir.resolve("smithyprojections/sdk-codegen-test/Cargo.toml").writeText(generateCargoWorkspace(CodegenTests))
}
}
registerGenerateSmithyBuildTask(rootProject, project, pluginName, allCodegenTests)
registerGenerateCargoWorkspaceTask(rootProject, project, pluginName, allCodegenTests, workingDirUnderBuildDir)

tasks["smithyBuildJar"].dependsOn("generateSmithyBuild")
tasks["assemble"].finalizedBy("generateCargoWorkspace")

tasks.register<Exec>("cargoCheck") {
workingDir("build/smithyprojections/sdk-codegen-test/")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir("build/smithyprojections/sdk-codegen-test/")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("cargoDocs") {
workingDir("build/smithyprojections/sdk-codegen-test/")
environment("RUSTDOCFLAGS", defaultRustDocFlags)
commandLine("cargo", "doc", "--no-deps")
dependsOn("assemble")
}

tasks.register<Exec>("cargoClippy") {
workingDir("build/smithyprojections/sdk-codegen-test/")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}
registerModifyMtimeTask(project)
registerCargoCommandsTasks(project, buildDir.resolve(workingDirUnderBuildDir), defaultRustDocFlags)

tasks["test"].finalizedBy("cargoCheck", "cargoClippy", "cargoTest", "cargoDocs")
tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString })

tasks["clean"].doFirst {
delete("smithy-build.json")
}
tasks["clean"].doFirst { delete("smithy-build.json") }
32 changes: 3 additions & 29 deletions aws/sdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ configure<software.amazon.smithy.gradle.SmithyExtension> {
}

val smithyVersion: String by project
val defaultRustFlags: String by project
val defaultRustDocFlags: String by project
val properties = PropertyRetriever(rootProject, project)

Expand Down Expand Up @@ -388,35 +387,10 @@ tasks["assemble"].apply {
finalizedBy("finalizeSdk")
}

tasks.register<Exec>("cargoCheck") {
workingDir(outputDir)
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check", "--lib", "--tests", "--benches")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir(outputDir)
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("cargoDocs") {
workingDir(outputDir)
environment("RUSTDOCFLAGS", defaultRustDocFlags)
commandLine("cargo", "doc", "--no-deps", "--document-private-items")
dependsOn("assemble")
}

tasks.register<Exec>("cargoClippy") {
workingDir(outputDir)
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}
registerCargoCommandsTasks(project, outputDir, defaultRustDocFlags)
registerGenerateCargoConfigTomlTask(project, outputDir)

tasks["test"].finalizedBy("cargoClippy", "cargoTest", "cargoDocs")
tasks["test"].finalizedBy(Cargo.CLIPPY, Cargo.TEST, Cargo.DOCS)

tasks.register<Delete>("deleteSdk") {
delete = setOf(outputDir)
Expand Down
139 changes: 138 additions & 1 deletion buildSrc/src/main/kotlin/CodegenTestCommon.kt
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import java.lang.IllegalArgumentException
import org.gradle.api.Project
import org.gradle.api.tasks.Exec
import org.gradle.kotlin.dsl.extra
import org.gradle.kotlin.dsl.register
import java.io.File

/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
Expand Down Expand Up @@ -76,6 +80,7 @@ fun codegenTests(properties: PropertyRetriever, allTests: List<CodegenTest>): Li
}

val AllCargoCommands = listOf(Cargo.CHECK, Cargo.TEST, Cargo.CLIPPY, Cargo.DOCS)

/**
* Filter the Cargo commands to be run on the generated Rust crates using the given [properties].
* The list of Cargo commands that is run by default is defined in [AllCargoCommands].
Expand All @@ -102,3 +107,135 @@ fun cargoCommands(properties: PropertyRetriever): List<Cargo> {
}
return ret
}

fun registerGenerateSmithyBuildTask(
rootProject: Project,
project: Project,
pluginName: String,
allCodegenTests: List<CodegenTest>
) {
val properties = PropertyRetriever(rootProject, project)
project.tasks.register("generateSmithyBuild") {
description = "generate smithy-build.json"
outputs.file(project.projectDir.resolve("smithy-build.json"))

doFirst {
project.projectDir.resolve("smithy-build.json")
.writeText(
generateSmithyBuild(
rootProject.projectDir.absolutePath,
pluginName,
codegenTests(properties, allCodegenTests)
)
)

// If this is a rebuild, cache all the hashes of the generated Rust files. These are later used by the
// `modifyMtime` task.
project.extra["previousBuildHashes"] = project.buildDir.walk()
.filter { it.name.endsWith(".rs") || it.name == "Cargo.toml" || it.name == "Cargo.lock" }
.map {
HashUtils.getCheckSumFromFile(it) to it.lastModified()
}
.toMap()
}
}
}

fun registerGenerateCargoWorkspaceTask(
rootProject: Project,
project: Project,
pluginName: String,
allCodegenTests: List<CodegenTest>,
workingDirUnderBuildDir: String
) {
val properties = PropertyRetriever(rootProject, project)
project.tasks.register("generateCargoWorkspace") {
description = "generate Cargo.toml workspace file"
doFirst {
project.buildDir.resolve("$workingDirUnderBuildDir/Cargo.toml")
.writeText(generateCargoWorkspace(pluginName, codegenTests(properties, allCodegenTests)))
}
}
}

fun registerGenerateCargoConfigTomlTask(
project: Project,
outputDir: File
) {
project.tasks.register("generateCargoConfigToml") {
description = "generate `.cargo/config.toml`"
doFirst {
outputDir.resolve(".cargo").mkdir()
outputDir.resolve(".cargo/config.toml")
.writeText(
"""
[build]
rustflags = ["--deny", "warnings"]
""".trimIndent()
)
}
}
}

fun registerModifyMtimeTask(
project: Project
) {
// Cargo uses `mtime` (among other factors) to determine whether a compilation unit needs a rebuild. While developing,
// it is likely that only a small number of the generated crate files are modified across rebuilds. This task compares
// the hashes of the newly generated files with the (previously cached) old ones, and restores their `mtime`s if the
// hashes coincide.
// Debugging tip: it is useful to run with `CARGO_LOG=cargo::core::compiler::fingerprint=trace` to learn why Cargo
// determines a compilation unit needs a rebuild.
// For more information see https://github.com/awslabs/smithy-rs/issues/1412.
project.tasks.register("modifyMtime") {
description = "modify Rust files' `mtime` if the contents did not change"
dependsOn("generateSmithyBuild")

doFirst {
val previousBuildHashes: Map<String, Long> = project.extra["previousBuildHashes"] as Map<String, Long>

project.buildDir.walk()
.filter { it.name.endsWith(".rs") || it.name == "Cargo.toml" }
.map {
HashUtils.getCheckSumFromFile(it) to it
}
.forEach { (currentHash, currentFile) ->
previousBuildHashes[currentHash]?.also { oldMtime ->
println("Setting `mtime` of $currentFile back to `$oldMtime` because its hash `$currentHash` remained unchanged after a rebuild.")
currentFile.setLastModified(oldMtime)
}
}
}
}
}

fun registerCargoCommandsTasks(
project: Project,
outputDir: File,
defaultRustDocFlags: String
) {
project.tasks.register<Exec>(Cargo.CHECK.toString) {
dependsOn("assemble", "modifyMtime", "generateCargoConfigToml")
workingDir(outputDir)
commandLine("cargo", "check", "--lib", "--tests", "--benches")
}

project.tasks.register<Exec>(Cargo.TEST.toString) {
dependsOn("assemble", "modifyMtime", "generateCargoConfigToml")
workingDir(outputDir)
commandLine("cargo", "test")
}

project.tasks.register<Exec>(Cargo.DOCS.toString) {
dependsOn("assemble", "modifyMtime", "generateCargoConfigToml")
workingDir(outputDir)
environment("RUSTDOCFLAGS", defaultRustDocFlags)
commandLine("cargo", "doc", "--no-deps", "--document-private-items")
}

project.tasks.register<Exec>(Cargo.CLIPPY.toString) {
dependsOn("assemble", "modifyMtime", "generateCargoConfigToml")
workingDir(outputDir)
commandLine("cargo", "clippy")
}
}
Loading

0 comments on commit 668fbb4

Please sign in to comment.