KTOR-7667: Enable Gradle configuration cache #4741
Conversation
WalkthroughThe changes update several Gradle build scripts and settings. In two build script files, OS retrieval logic in task conditions was refactored to improve readability. Additional logic was introduced in the Kotlin Native compilation configuration to disable the configuration cache on certain tasks. New files were added to handle configuration cache workarounds using reflection and to apply cache-related settings. Properties in the Gradle configuration were updated to enable cache features and isolated projects. Finally, a test server variable was moved into a local configuration scope, and the settings file was updated to enable a feature preview and include a new caching plugin. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
osipxd
force-pushed
the
osipxd/configuration-cache
branch
from
f312b4c to
22e82df
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
build-logic/src/main/kotlin/ktorbuild/CInterop.kt (1)
67-71: Add clarifying inline commentary on the workaround.The workaround marking certain tasks as incompatible with the configuration cache is valid. For maintainability, consider adding a direct reference comment to the KT-76147 or linking the issue for future context. This ensures that the reasoning behind the compatibility limitation is clear to other contributors.
build-settings-logic/src/main/kotlin/ConfigurationCacheWorkarounds.kt (1)
8-35: Reflection-based field replacement: handle potential security or tight coupling.Reflection with private fields can break if internal implementations in Gradle or the JVM change. Make sure to provide clear warnings or guard checks if the reflection fails. Regularly test to ensure that future Gradle updates do not break this.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts(1 hunks)build-logic/src/main/kotlin/ktorbuild.publish.gradle.kts(1 hunks)build-logic/src/main/kotlin/ktorbuild/CInterop.kt(2 hunks)build-settings-logic/src/main/kotlin/ConfigurationCacheWorkarounds.kt(1 hunks)build-settings-logic/src/main/kotlin/ktorbuild.configuration-cache.settings.gradle.kts(1 hunks)gradle.properties(2 hunks)ktor-test-server/src/main/kotlin/test-server.gradle.kts(1 hunks)settings.gradle.kts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
build-logic/src/main/kotlin/ktorbuild.publish.gradle.kts (1)
build-logic/src/main/kotlin/ktorbuild/internal/publish/PublishTasks.kt (1) (1)
isAvailableForPublication(30-44)
🔇 Additional comments (11)
ktor-test-server/src/main/kotlin/test-server.gradle.kts (1)
7-12: Limit Variable Scope for Test Server Service
Moving the declaration oftestServerServiceinside thetasks.withType<AbstractTestTask>().configureEachblock properly limits its scope to only where it’s needed. This improves clarity and helps prevent unintended misuse elsewhere in the build script.build-logic/src/main/kotlin/ktorbuild.publish.gradle.kts (1)
60-62: Refactor OS Retrieval for Improved Readability
ExtractingktorBuild.os.get()into a local variable (os) within theonlyIfblock reduces redundant calls and enhances readability. This straightforward change aligns with the broader refactoring effort observed elsewhere.settings.gradle.kts (2)
5-6: Enable Stable Configuration Cache Feature Preview
The addition ofenableFeaturePreview("STABLE_CONFIGURATION_CACHE")activates the stable configuration cache feature in Gradle, which is in line with the objective of this PR. Please ensure that your build environment supports this feature preview.
15-15: Add Ktorbuild Configuration Cache Plugin
Including thektorbuild.configuration-cacheplugin integrates the cache-related settings into the build. Confirm that this plugin is compatible with your existing build tools and that its behavior meets the overall caching objectives.build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts (1)
60-71: Refine Native Test Task Conditions with Local OS Variable
For the native targets, extracting the operating system value into a local variable (os) and using it to conditionally enable tasks (e.g.,"linkDebugTestLinuxX64","linkDebugTestLinuxArm64","linkDebugTestMingwX64") improves both efficiency and readability. This guarantees that tests run only on appropriate hosts.build-settings-logic/src/main/kotlin/ktorbuild.configuration-cache.settings.gradle.kts (1)
1-24: Introduce Configuration Cache Workaround Script
This new script adeptly applies a workaround for KT-72933 by:
• Ignoring specific bean fields that lead to out-of-memory issues.
• Marking tasks associated withMetadataDependencyTransformationTaskas incompatible with the configuration cache.The implementation is clear and well-commented. Please verify that the workaround aligns with the latest Gradle configuration cache limitations and that no downstream tasks are unintentionally affected.
build-logic/src/main/kotlin/ktorbuild/CInterop.kt (1)
11-15: Imports look appropriate for new Kotlin Native compilation logic.These additions properly set up the project to detect and manage
KotlinNativeCompiletasks viawithType. They appear necessary and do not introduce any obvious issues.gradle.properties (2)
59-61: Keep the TODO and remove once isolated projects are fully enabled.Using
kotlin.kmp.isolated-projects.support=enableis a good preparatory step. The inline TODO note is a helpful reminder to remove this once the Gradle feature matures. No further changes needed here.
46-47:❓ Verification inconclusive
Confirm tasks remain safe under parallel configuration cache.
Enabling both configuration cache and parallel execution can surface concurrency-related pitfalls in plugin or task implementations. Please verify that all relevant tasks and plugins are thread-safe to avoid unexpected issues.
🏁 Script executed:
#!/bin/bash # This script gathers tasks that implement parallelizable changes. # We'll look for tasks referencing concurrency or shared states. rg -A 3 "implements.*Parallel" rg -A 3 "shared.*state"Length of output: 2285
Parallel Configuration Cache: Please Verify Thread-Safety of Custom Tasks
The grep searches only revealed expected shared state handling in the Ktor client modules (e.g., in
HttpResponsePipeline.ktandHttpRequestPipeline.kt). No additional unsafe implementations were detected from searching for "implements.*Parallel".
- Confirm that any custom tasks or plugins that aren’t part of these well-reviewed libraries are also thread-safe.
- Revisit any tasks using shared mutable state to ensure proper synchronization is in place.
build-settings-logic/src/main/kotlin/ConfigurationCacheWorkarounds.kt (2)
37-43:❓ Verification inconclusive
Double-check concurrency implications of mutating static fields.
The
setFinalStaticapproach is unusual and can be risky in multi-threaded builds. Currently, the code attempts a safe fallback if the direct field replacement fails, but consider locking mechanisms or verifying that parallel builds won't cause intermediate inconsistent states.
🏁 Script executed:
#!/bin/bash #!/bin/bash # Attempt to locate concurrent usage or references to setFinalStatic in this codebase. rg -A 5 "setFinalStatic"Length of output: 1461
Double-check concurrency implications:
After verifying the usage ofsetFinalStaticacross the codebase, no explicit locking or synchronization mechanisms were found around its invocation. The fallback for modifying array elements is also unsynchronized. Please ensure that these operations are safe from race conditions during parallel builds or concurrent access, or consider adding appropriate locking where necessary.
5-7:❓ Verification inconclusive
Use of
sun.misc.Unsaferaises portability concerns.While
Unsafeis powerful, it can cause compatibility issues on newer Java versions and alternative JVMs. Consider verifying that the usage is supported in all deployment environments.
🏁 Script executed:
#!/bin/bash # Check if this codebase references additional internal or JDK-specific APIs that could be removed or replaced. rg -A 5 "sun\.misc\.Unsafe"Length of output: 694
Action Required: Verify Compatibility of Internal Unsafe API Usage
The file
build-settings-logic/src/main/kotlin/ConfigurationCacheWorkarounds.kt(around lines 5–7) imports and usessun.misc.Unsafe. This internal API could lead to portability and compatibility issues on newer Java versions or alternative JVMs. Please verify that its usage is supported in all of your target deployment environments. If possible, consider alternatives or document the environment constraints explicitly.
| @@ -43,6 +43,8 @@ doctor.enableTaskMonitoring=false | |||
| org.gradle.daemon=true | |||
| org.gradle.caching=true | |||
| org.gradle.parallel=true | |||
| org.gradle.configuration-cache=true | |||
| org.gradle.configuration-cache.parallel=true | |||
There was a problem hiding this comment.
Probably we should also enable "configuration-on-demand" by default, but I'm not sure about it as this mode might silently break something. I would propose enabling this property locally and after couple of weeks we will add it here if everything goes okay.
org.gradle.configureondemand =true
|
You might have broken some tests by making the build run twice as fast... 😅 |
osipxd
force-pushed
the
osipxd/configuration-cache
branch
from
22e82df to
009b5f3
Compare
osipxd
force-pushed
the
osipxd/configuration-cache
branch
from
009b5f3 to
eeaaf33
Compare
osipxd
force-pushed
the
osipxd/configuration-cache
branch
from
eeaaf33 to
21905d3
Compare
Subsystem
Infrastructure
Motivation
KTOR-7667 Enable Gradle configuration cache
Solution
Finally, I've found a hackaround for KT-72933, and we can enable configuration cache 🎉
It will work only for tasks not involving
MetadataDependencyTransformationTask.Some benchmark results for scenario "Rerun
:ktor-client:ktor-client-core:jvmTestafter a change in JVM source set."default - optimizations disabled
COD - configuration on demand enabled
CC - configuration cache enabled
Scenarios
defaultandCODrepresent the case when there is no configuration cache that could be reused, let's call it "initial configuration".Results show that configuration on demand significantly improves initial configuration configuration times and slightly improves storing and loading of configuration cache (because of smaller cache size)
Next steps: