Skip to content

Comments

Optimize coverage executions#28323

Open
anonrig wants to merge 1 commit intobazelbuild:masterfrom
anonrig:yagiz/optimize-coverage-exec
Open

Optimize coverage executions#28323
anonrig wants to merge 1 commit intobazelbuild:masterfrom
anonrig:yagiz/optimize-coverage-exec

Conversation

@anonrig
Copy link
Contributor

@anonrig anonrig commented Jan 16, 2026

  • Replaces hard-coded 4 threads to 16 for coverage file parsing parallelism.
  • Realized that forkjoinpool resource was not properly cleaned up.
  • Parsed different file types in parallel rather than sequential.
  • Optimized directory walking.

@anonrig anonrig requested a review from lberki as a code owner January 16, 2026 16:42
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Jan 16, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant performance optimizations by leveraging parallel processing for coverage file parsing and directory walking. The changes include dynamically adjusting the parallelism level based on available processors, using CompletableFuture to parse different file types concurrently, and ensuring proper cleanup of ForkJoinPool resources. Additionally, the filtering logic for coverage files has been extracted into a dedicated method, improving code readability and maintainability. Overall, these are valuable improvements for the tool's efficiency.

@anonrig anonrig force-pushed the yagiz/optimize-coverage-exec branch from 721ce12 to c36f4d7 Compare January 16, 2026 16:47
@meisterT meisterT requested a review from c-mita January 19, 2026 11:12
@c-mita
Copy link
Member

c-mita commented Jan 19, 2026

Have you got any tests that can quantify the impact of these changes?

Replaces hard-coded 4 threads to 16 for coverage file parsing parallelism.

When making changes relating to the memory usage of the coverage merger, I noticed that increasing parallelism didn't really appear to change much, and in the context of very large reports, could actually make things worse.

.orElse(Coverage.create()))
.get();
try {
// Process each file as a separate task to leverage work-stealing.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many parsed coverage reports can be "in-flight" at a time with this approach?

Previously it was just double the number of threads; each thread had the "accumulated" report and the currently parsed one. I'm not sure that's the case here.

@iancha1992 iancha1992 added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jan 22, 2026
@meteorcloudy meteorcloudy added team-Rules-CPP Issues for C++ rules and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer coverage team-Rules-CPP Issues for C++ rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants