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
There was a problem hiding this comment.
🤔 Do we cover this by integTest?
There was a problem hiding this comment.
We do with a unit test:
Also, integration tests had to be updated to reflect this, including a smoke test.
| 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
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