Skip to content

Commit f17e609

Browse files
nav-navSpace Team
authored and
Space Team
committedMar 5, 2025
Avoid multiple finalizations of generalConfigurationMetrics
Removed the finalization of BuildFusService.generalConfigurationMetrics in FinalizeConfigurationFusMetricAction to prevent the repeated finalization that could occur with each sub-project. #KT-73842 Verification Pending (cherry picked from commit 5ed58ce)
1 parent 45e81bb commit f17e609

File tree

7 files changed

+84
-7
lines changed

7 files changed

+84
-7
lines changed
 

‎libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/FusStatisticsIT.kt

+52-3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import kotlin.io.path.deleteIfExists
1818
import kotlin.io.path.writeText
1919
import kotlin.io.path.deleteRecursively
2020
import kotlin.io.path.listDirectoryEntries
21+
import kotlin.test.assertEquals
2122
import kotlin.test.assertTrue
2223

2324
@DisplayName("FUS statistic")
@@ -515,7 +516,8 @@ class FusStatisticsIT : KGPBaseTest() {
515516
| }
516517
| }
517518
|}
518-
""".trimMargin())
519+
""".trimMargin()
520+
)
519521

520522
build("linkDebugExecutableHost", "-Pkotlin.session.logger.root.path=$projectPath") {
521523
assertFileContains(
@@ -537,13 +539,60 @@ class FusStatisticsIT : KGPBaseTest() {
537539
//Test uses deprecated Gradle features
538540
project("multiplatformFlowAction", gradleVersion, buildOptions = defaultBuildOptions.copy(warningMode = WarningMode.Summary)) {
539541
buildScriptInjection {
540-
project.tasks.register("doNothing"){}
542+
project.tasks.register("doNothing") {}
541543
}
542-
543544
build("doNothing")
544545
}
545546
}
546547

548+
@DisplayName("add configuration metrics after build was finish")
549+
@GradleTest
550+
@JvmGradlePluginTests
551+
@GradleTestVersions(
552+
minVersion = TestVersions.Gradle.G_8_2,
553+
)
554+
fun concurrencyModificationExceptionTest(gradleVersion: GradleVersion) {
555+
val rounds = 100
556+
project(
557+
"multiClassloaderProject", gradleVersion,
558+
) {
559+
repeat(rounds) {
560+
build(
561+
"compileKotlin", "-Pkotlin.session.logger.root.path=$projectPath", "-Dorg.gradle.parallel=true",
562+
buildOptions = defaultBuildOptions.copy(
563+
buildReport = listOf(BuildReportType.FILE),
564+
isolatedProjects = IsolatedProjectsMode.ENABLED,
565+
),
566+
) {
567+
assertOutputDoesNotContain("BuildFusService was not registered")
568+
}
569+
570+
build("clean", buildOptions = buildOptions)
571+
}
572+
573+
val fusReports = baseFusStatisticsDirectory.listDirectoryEntries()
574+
assertEquals(getExpectedFusFilesCount(gradleVersion, rounds), fusReports.size)
575+
576+
fusReports.forEach { path ->
577+
assertFileContains(
578+
path,
579+
"CONFIGURATION_IMPLEMENTATION_COUNT",
580+
"NUMBER_OF_SUBPROJECTS",
581+
)
582+
}
583+
}
584+
}
585+
586+
private fun getExpectedFusFilesCount(gradleVersion: GradleVersion, rounds: Int): Int {
587+
val expectedFiles = if (gradleVersion >= GradleVersion.version(TestVersions.Gradle.G_8_9)) {
588+
//every submodule will create a separate file. There are two modules in the project
589+
rounds * 2
590+
} else {
591+
rounds
592+
}
593+
return expectedFiles
594+
}
595+
547596
private fun TestProject.applyDokka(version: String) {
548597
buildGradle.replaceText(
549598
"plugins {",

‎libraries/tools/kotlin-gradle-plugin/api/all/kotlin-gradle-plugin.api

+1
Original file line numberDiff line numberDiff line change
@@ -2585,6 +2585,7 @@ public abstract interface class org/jetbrains/kotlin/gradle/plugin/statistics/Bu
25852585
public abstract fun getBuildId ()Lorg/gradle/api/provider/Property;
25862586
public abstract fun getBuildStatisticsConfiguration ()Lorg/gradle/api/provider/Property;
25872587
public abstract fun getGeneralConfigurationMetrics ()Lorg/gradle/api/provider/Property;
2588+
public abstract fun getGeneralMetricsFinalized ()Lorg/gradle/api/provider/Property;
25882589
}
25892590

25902591
public abstract class org/jetbrains/kotlin/gradle/plugin/statistics/CloseActionBuildFusService : org/jetbrains/kotlin/gradle/plugin/statistics/BuildFusService {

‎libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/statistics/BuildFusService.kt

+23-1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ abstract class BuildFusService<T : BuildFusService.Parameters> :
5858
}
5959

6060
interface Parameters : BuildServiceParameters {
61+
val generalMetricsFinalized: Property<Boolean>
6162
val generalConfigurationMetrics: Property<MetricContainer>
6263
val buildStatisticsConfiguration: Property<KotlinBuildStatsConfiguration>
6364
val buildId: Property<String>
@@ -83,6 +84,18 @@ abstract class BuildFusService<T : BuildFusService.Parameters> :
8384
internal val serviceName = "${BuildFusService::class.simpleName}_${BuildFusService::class.java.classLoader.hashCode()}"
8485
private var buildStartTime: Long = System.currentTimeMillis()
8586

87+
internal fun getBuildFusService(project: Project) =
88+
if (project.buildServiceShouldBeCreated) {
89+
project.gradle.sharedServices.registrations.findByName(serviceName).also {
90+
if (it == null) {
91+
project.logger.info("BuildFusService was not registered")
92+
}
93+
}
94+
} else {
95+
null
96+
}
97+
98+
8699
fun registerIfAbsent(project: Project, pluginVersion: String, buildUidService: Provider<BuildUidService>) =
87100
if (project.buildServiceShouldBeCreated) {
88101
registerIfAbsentImpl(project, pluginVersion, buildUidService).also { serviceProvider ->
@@ -224,4 +237,13 @@ class MetricContainer : Serializable {
224237
}
225238

226239
private val Project.buildServiceShouldBeCreated
227-
get() = !isInIdeaSync.get() && kotlinPropertiesProvider.enableFusMetricsCollection
240+
get() = !isInIdeaSync.get() && kotlinPropertiesProvider.enableFusMetricsCollection
241+
242+
internal fun BuildFusService.Parameters.finalizeGeneralConfigurationMetrics() {
243+
if (generalMetricsFinalized.get()) return
244+
synchronized(this) {
245+
if (generalMetricsFinalized.get()) return
246+
generalMetricsFinalized.set(true)
247+
generalConfigurationMetrics.finalizeValue()
248+
}
249+
}

‎libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/statistics/CloseActionBuildFusService.kt

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ abstract class CloseActionBuildFusService:
2020
): Provider<CloseActionBuildFusService> {
2121
return project.gradle.sharedServices.registerIfAbsent(serviceName, CloseActionBuildFusService::class.java) { spec ->
2222
spec.parameters.generalConfigurationMetrics.set(generalConfigurationMetricsProvider)
23+
spec.parameters.generalMetricsFinalized.set(false)
2324
spec.parameters.buildStatisticsConfiguration.set(KotlinBuildStatsConfiguration(project))
2425
spec.parameters.buildId.value(buildUidService.map { it.buildId }).disallowChanges()
2526
//init value to avoid `java.lang.IllegalStateException: GradleScopeServices has been closed` exception on close

‎libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/statistics/ConfigurationMetricParameterFlowActionBuildFusService.kt

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ abstract class ConfigurationMetricParameterFlowActionBuildFusService() : BuildFu
2323
ConfigurationMetricParameterFlowActionBuildFusService::class.java
2424
) { spec ->
2525
spec.parameters.generalConfigurationMetrics.set(generalConfigurationMetricsProvider)
26+
spec.parameters.generalMetricsFinalized.set(false)
2627
spec.parameters.buildStatisticsConfiguration.set(KotlinBuildStatsConfiguration(project))
2728
spec.parameters.buildId.value(buildUidService.map { it.buildId }).disallowChanges()
2829
//init value to avoid `java.lang.IllegalStateException: GradleScopeServices has been closed` exception on close

‎libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/statistics/FinalizeConfigurationFusMetricAction.kt

+5-3
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,22 @@ import org.jetbrains.kotlin.gradle.plugin.await
1313
internal val FinalizeConfigurationFusMetricAction = KotlinProjectSetupCoroutine {
1414
KotlinPluginLifecycle.Stage.ReadyForExecution.await()
1515

16-
project.gradle.sharedServices.registrations.findByName(BuildFusService.serviceName)?.also {
16+
BuildFusService.getBuildFusService(project)?.also {
1717
val parameters = it.parameters
1818
if (parameters is ConfigurationMetricsBuildFusParameters) {
1919
//build service parameter is used,
2020
//it is important to avoid service parameters initialization before all configuration metrics are collected
21-
parameters.generalConfigurationMetrics.finalizeValue()
21+
parameters.finalizeGeneralConfigurationMetrics()
2222
parameters.configurationMetrics.add(KotlinProjectConfigurationMetrics.collectMetrics(project))
2323
} else {
24-
(parameters as BuildFusService.Parameters).generalConfigurationMetrics.finalizeValue()
24+
(parameters as BuildFusService.Parameters).finalizeGeneralConfigurationMetrics()
2525

2626
//build service field is used,
2727
//it is safe to access build service, as configuration metrics will be cached in [BuildFinishFlowAction]
2828
val buildFusService = it.service.orNull as FlowActionBuildFusService
2929
buildFusService.addConfigurationTimeMetric(KotlinProjectConfigurationMetrics.collectMetrics(project))
3030
}
3131
}
32+
33+
3234
}

‎libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/statistics/FlowActionBuildFusService.kt

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ abstract class FlowActionBuildFusService @Inject constructor(
3434
): Provider<out BuildFusService<out Parameters>> {
3535
return project.gradle.sharedServices.registerIfAbsent(serviceName, FlowActionBuildFusService::class.java) { spec ->
3636
spec.parameters.generalConfigurationMetrics.set(generalConfigurationMetricsProvider)
37+
spec.parameters.generalMetricsFinalized.set(false)
3738
spec.parameters.buildStatisticsConfiguration.set(KotlinBuildStatsConfiguration(project))
3839
spec.parameters.buildId.value(buildUidService.map { it.buildId }).disallowChanges()
3940
}.also { buildService ->

0 commit comments

Comments
 (0)