Acquire project locks in component state #29193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@bot-gradle test apt |
|
I've triggered the following builds for you. Click here to see all build failures. |
|
@bot-gradle test this |
|
I've triggered the following builds for you. Click here to see all build failures. |
jvandort
force-pushed
the
jvandort/dm/move-stateful-logic-from-local-metadata-to-state
branch
from
4491d91
to
1ac7e5e
Compare
jvandort
force-pushed
the
jvandort/dm/acquire-project-locks-in-component-state
branch
from
ea565aa
to
548c896
Compare
Base automatically changed from
jvandort/dm/move-stateful-logic-from-local-metadata-to-state
to
master
May 22, 2024 20:43
jvandort
commented
May 22, 2024
...n/java/org/gradle/internal/component/local/model/DefaultLocalComponentGraphResolveState.java
Show resolved
Hide resolved
big-guy
approved these changes
May 29, 2024
...n/java/org/gradle/internal/component/local/model/DefaultLocalComponentGraphResolveState.java
Show resolved
Hide resolved
Previously, project locks were not acquired in the component graph state when project state was being accessed. This led to some situations where data races could occur. In many cases, when dependency resolution is properly declared, this does not matter, as walking the task dependency graph is single-threaded, and therefore these caches would be realized without any data races possible. However, when the parallel flag is enabled, for undeclared dependency resolution, or other potential edge cases, it was still possible to get data races when multiple resolutions both access project state simultaneously. This change makes sure that all project state is wrapped in the proper locks before it is accessed. This should also clear the way to making task graph calculation multithreaded, as resolving configurations during task dependency graph calculation is currently a major bottleneck of configuration-time.
jvandort
force-pushed
the
jvandort/dm/acquire-project-locks-in-component-state
branch
from
548c896
to
6c71b73
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, project locks were not acquired in the component graph state when project state was being accessed. This led to some situations where data races could occur.
In many cases, when dependency resolution is properly declared, this does not matter, as walking the task dependency graph is single-threaded, and therefore these caches would be realized without any data races possible. However, when the parallel flag is enabled, for undeclared dependency resolution, or other potential edge cases, it was still possible to get data races when multiple resolutions both access project state simultaneously.
This change makes sure that all project state is wrapped in the proper locks before it is accessed.
This should also clear the way to making task graph calculation multithreaded, as resolving configurations during task dependency graph calculation is currently a major bottleneck of configuration-time.
Context
Contributor Checklist
<subproject>/src/integTest) to verify changes from a user perspective.<subproject>/src/test) to verify logic../gradlew sanityCheck../gradlew <changed-subproject>:quickTest.Reviewing cheatsheet
Before merging the PR, comments starting with