Skip to content

Commit f17e609

Browse files
nav-navSpace Team
authored and
Space Team
committed
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)