-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modularize :configuration-cache
#29207
Conversation
5c67a69
to
93425a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good step in the right direction.
Only actual changes I could see were the introcdution of a base ProblemsListener
implementation (AbstractProblemsListener
), and introduction of a sealed class hierarchy for IsolateOwner
implementations.
suspend fun WriteContext.writeStateOf(bean: Any) | ||
dependencies { | ||
api(libs.futureKotlin("stdlib")) | ||
implementation(projects.javaLanguageExtensions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I see the parallel between this new module and the existing java-language-extensions
, I have no issue with the new name. Actually, I think this module is a much better described as "stdlib-extensions" than the one for Java, as the latter actually introduces things that go way beyond providing general purpose extensions to the Java library.
It seems we are consuming java-language-extensions
just to get access to Cast
. That is fine, more of a problem with that module's purpose than with this one (and that module's scope is already an improvement over recent past).
api(projects.serviceProvider) | ||
api(projects.configurationProblemsBase) | ||
api(project(":base-services")) | ||
api(project(":build-operations")) | ||
// TODO - it might be good to allow projects to contribute state to save and restore, rather than have this project know about everything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this refers to the design you mentioned we want to pursue, which would de-invert the dependencies.
@@ -98,56 +89,10 @@ class DefaultWriteContext( | |||
} | |||
|
|||
|
|||
internal | |||
class LoggingTracer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"deleted" classes here were moved to their own files
import kotlin.coroutines.Continuation | ||
import kotlin.coroutines.CoroutineContext | ||
import kotlin.coroutines.coroutineContext | ||
import kotlin.coroutines.startCoroutine | ||
import kotlin.coroutines.suspendCoroutine | ||
|
||
|
||
internal | ||
fun <T> singleton(value: T): Codec<T> = | ||
SingletonCodec(value) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functions were moved to platforms/core-configuration/configuration-cache/src/main/kotlin/org/gradle/configurationcache/serialization/Unsupported.kt
@@ -275,104 +219,34 @@ suspend fun <K, V, T : MutableMap<K, V>> ReadContext.readMapEntriesInto(items: T | |||
} | |||
|
|||
|
|||
internal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extension functions moved to platforms/core-configuration/configuration-cache/src/main/kotlin/org/gradle/configurationcache/serialization/ClassPathEncodingExtensions.kt
@@ -110,39 +111,12 @@ interface IsolateContext { | |||
} | |||
|
|||
|
|||
sealed class IsolateOwner { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to own file and made part of a new sealed class hierachy
@@ -469,7 +474,7 @@ class ConfigurationCacheState( | |||
|
|||
build.createProjects() | |||
|
|||
initProjectProvider(build::getProject) | |||
setSingletonProperty<ProjectProvider>(build::getProject) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to decouple WriteContext
from ProjectInternal
The merge queue build has failed. Click here to see all failures. |
72ccc61
to
761e557
Compare
The merge queue build has failed. Click here to see all failures. |
a8e067e
to
a5c6b2c
Compare
@bot-gradle test AST |
I've triggered the following builds for you. Click here to see all build failures. |
Initially by extracting three modules: - configuration-problems-base: base utilities for reporting configuration problems with structured messages and property traces - object-graph-serialization: aimed at holding the core configuration cache serialization implementation but currently only containing the detangled code - kotlin-language-extensions: holding the Kotlin extensions shared by all modules This is just the first step, subsequent commits will detangle and move more code from `:configuration-cache` into `:object-graph-serialization` and rename all moved packages to match the new structure.
Because it is used by `DefaultProblemsFactory` from `:configuration-problems-base`.
To `:object-graph-serialization`.
`graph` was redundant.
With a potential impact to `gcc2speedscope` since generated type names are no longer unpacked to avoid a dependency on `:base-services`.
a5c6b2c
to
dd1e518
Compare
Since the `kotlin` word in module names is reserved for Kotlin DSL modules.
dd1e518
to
8a941fd
Compare
Initially by extracting three modules:
:configuration-problems-base
: base utilities for reporting configuration problems with structured messages and property traces:object-serialization
: core configuration cache serialization api and implementation but no codecs:java-language-extensions-kt
: Java/Kotlin stdlib extensions shared by all modulesThis is just the first step, subsequent PRs will detangle and move more code from
:configuration-cache
into:object-serialization
and rename all moved packages to match the new structure.Context
:configuration-cache
is currently one of the largest Gradle modules. It is time to modularize it to reduce compilation times and cognitive overhead.Reviewing cheatsheet
Before merging the PR, comments starting with