Improve codeowners lookup performance#12058
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: b45a9a7 | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
@codex review |
|
/datadog autotest review |
There was a problem hiding this comment.
More details
The adaptive index retains the key lookup invariants on inspection: unsafe-prefix rules stay in the fallback list, and source order decides between fallback and keyed matches. Execution could not complete because Gradle 9.6.1 is not cached and the sandbox blocks the wrapper download.
🤖 Datadog Autotest · Commit b45a9a7 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
More details
Large-rule-set matching preserved the prior implementation across directory, recursive, wildcard, fixed-prefix, and malformed-separator shapes. Indexed ownership/exclusion lookup and fallback precedence also returned the expected owners, so no user-facing regression was identified.
📊 Validated against 101 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit b45a9a7 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b45a9a70b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (firstSeparator >= 0) { | ||
| return new String(c, prefixStart, position - prefixStart); |
There was a problem hiding this comment.
Keep trailing-slash rules out of the fixed-prefix index
In large sections where the index activates, a two-segment trailing-slash rule such as /services/api/ is bucketed under services/api, but the existing matcher still treats that rule as a plain prefix because no end-of-segment matcher is appended for trailing slashes. This makes behavior depend on file size: with 511 prior indexable entries services/api-v2/Test.java is matched by the linear scan, while with 512 entries the indexed lookup only checks the services/api-v2 bucket and returns no owner; exclusions have the same issue. Either keep these trailing-slash patterns in the fallback until the matcher semantics are tightened, or make the matcher and index agree.
Useful? React with 👍 / 👎.
What Does This Do
Improves CODEOWNERS lookup performance in Test Optimization.
The change removes several per-lookup allocations and adds adaptive indexing for large CODEOWNERS sections:
Stringinstead of creating achar[]The index activates only when a rule set contains at least 512 entries and at least half can be indexed. Smaller rule sets retain the linear lookup path. When the index activates, already parsed entries are moved into their buckets using index keys computed during parsing. Patterns are not reparsed.
Motivation
CODEOWNERS parsing happens once during repository initialization, but ownership lookup runs for every test and test suite. Large CODEOWNERS files can therefore consume measurable CPU and allocate hundreds of bytes per lookup.
The first implementation indexed every file. Benchmarks across Java projects showed that indexing small rule sets could add latency. The final implementation uses an adaptive gate so only large, indexable rule sets pay the indexing cost.
Additional Notes
The original benchmarking was performed against an input which contained 6,679 lines and 5,596 entries. Lookups were sampled up to 30,000 tracked paths.
Given the nature of the repository (multi-language monorepo), other projects were also benchmarked to account for specific patterns in codeowners definition for Java-centric repositories:
test-environment-trigger: skip
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]