Deduplicate configuration cache report problems #36047
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
03fa21f to
0f1ecef
Compare
| withProblem("Task `:problems` of type `BrokenTask`: cannot serialize object of type 'org.gradle.api.internal.project.DefaultProject', a subtype of 'org.gradle.api.Project', as these are not supported with the configuration cache.") | ||
| totalProblemsCount = 6 | ||
| problemsWithStackTraceCount = 2 | ||
| problemsWithStackTraceCount = 1 |
There was a problem hiding this comment.
these are unique counts now
| addNotReportedDegradingTasks() | ||
| addDegradingFeatures() | ||
| val summary = summarizer.get() | ||
| val hasNoProblemsForConsole = summary.reportableProblemCount == 0 |
There was a problem hiding this comment.
I got confused by reportableProblemCount - it really means a count of problems we report to the console
0f1ecef to
0b30add
Compare
| val lock = ReentrantLock() | ||
|
|
||
| fun get(): Summary = lock.withLock { | ||
| Summary( |
There was a problem hiding this comment.
Many parameters here have same type, wanted to make the data flows more explicit
| totalProblemCount = totalProblemCount, | ||
| uniqueProblemCount = problemCauses.size, | ||
| deferredProblemCount = deferredProblemCount, | ||
| consoleProblemCount = totalProblemCount - suppressedSilentlyProblemCount, |
There was a problem hiding this comment.
This is an actual change
| uniqueProblemCount = problemCauses.size, | ||
| deferredProblemCount = deferredProblemCount, | ||
| consoleProblemCount = totalProblemCount - suppressedSilentlyProblemCount, | ||
| consoleProblemCauses = ImmutableMap.copyOf(problemCauses.filterValues { it != ProblemSeverity.SuppressedSilently }), |
There was a problem hiding this comment.
So is this. Since we only care for problem causes we can show on the console, decided to filter it here, and reflect that in the property name
| overflowed = true | ||
| return false | ||
| } | ||
| if (severity != ProblemSeverity.SuppressedSilently) { |
There was a problem hiding this comment.
Before, we would not apply deduplication to problems with this severity, now we do
| fun onIncompatibleFeature() { | ||
| fun onIncompatibleFeature(problem: PropertyProblem) { | ||
| lock.withLock { | ||
| onProblem(problem, ProblemSeverity.SuppressedSilently) |
There was a problem hiding this comment.
For incompatible features, we were adding a problem to the report without letting the summarizer know - now we do report the problem so it is reflected on totalProblemCount and suppressedSilentlyProblemCount.
| * Returns whether problem was accepted. | ||
| */ | ||
| private | ||
| fun recordProblemCause(problem: PropertyProblem, severity: ProblemSeverity): Boolean { |
There was a problem hiding this comment.
Before, this may return false even if the new problem upgraded the severity of a previous one
be5eed3 to
fad4665
Compare
|
|
||
| assertThat( | ||
| "HTML report JS model has wrong number of total problem(s)", | ||
| numberOfProblems(), |
There was a problem hiding this comment.
this was relying on number of problems in the data - now that we deduplicate, we need to query the totalProblemCount slot instead
| } | ||
|
|
||
| private void assertProblemsHtmlReport(HasConfigurationCacheProblemsSpec spec) { | ||
| def totalProblemCount = spec.totalProblemsCount ?: spec.uniqueProblems.size() |
There was a problem hiding this comment.
this meant that if totalProblemsCount == 0 we would ignore it.
| ) | ||
|
|
||
| if (spec.checkReportProblems) { | ||
| def problemMessages = problemMessages().unique() |
There was a problem hiding this comment.
no need to remove duplicates here
This comment has been minimized.
This comment has been minimized.
|
|
||
| int getProblemsWithStackTrace() { | ||
| return problems.inject(0) { a, b -> a + (b.hasStackTrace ? b.count : 0) } | ||
| return problems.inject(0) { a, b -> a + (b.hasStackTrace ? 1 : 0) } |
There was a problem hiding this comment.
This count is unique now
fad4665 to
997b249
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The following builds have failed: |
| "Project ':' cannot access 'Project.plugins' functionality on subprojects via 'allprojects'", | ||
| "Project ':' cannot access 'Project.extensions' functionality on subprojects via 'allprojects'", | ||
| ) | ||
| // maximum number of problems we collect (should be 86520) |
There was a problem hiding this comment.
This was what made us notice the issue with lack of deduplication. I think 86520 was what I saw with a previous version of the init script used by this test which would fail to schedule some tasks. When that was addressed, and more tasks scheduled grew, so did the number of violations.
|
@bot-gradle test RFN |
This comment has been minimized.
This comment has been minimized.
|
The following builds have passed: |
ad6ef3d to
1ac66b7
Compare
Issue: #36046 (see reproducer there)
Without the fix:
Report size: 107M
With the fix:
Report size: 153K
Reviewing cheatsheet
Before merging the PR, comments starting with