Skip to content

Commit 9c335a6

Browse files
Improve codeowners lookup performance (#12058)
feat: improve codeowners lookup performance fix: keep trailing slash for linear scan Co-authored-by: daniel.mohedano <daniel.mohedano@datadoghq.com>
1 parent 5a8dcc6 commit 9c335a6

16 files changed

Lines changed: 282 additions & 64 deletions

File tree

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/CodeownersImpl.java

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44
import java.io.BufferedReader;
55
import java.io.IOException;
66
import java.io.Reader;
7-
import java.util.ArrayDeque;
87
import java.util.ArrayList;
98
import java.util.Collection;
109
import java.util.Collections;
11-
import java.util.Deque;
1210
import java.util.LinkedHashMap;
1311
import java.util.LinkedHashSet;
1412
import java.util.List;
@@ -20,9 +18,9 @@
2018

2119
public class CodeownersImpl implements Codeowners {
2220

23-
private final Collection<Section> sections;
21+
private final List<Section> sections;
2422

25-
private CodeownersImpl(Collection<Section> sections) {
23+
private CodeownersImpl(List<Section> sections) {
2624
this.sections = sections;
2725
}
2826

@@ -32,17 +30,25 @@ private CodeownersImpl(Collection<Section> sections) {
3230
*/
3331
@Override
3432
public @Nullable Collection<String> getOwners(@Nonnull String path) {
35-
char[] pathCharacters = path.toCharArray();
33+
if (sections.size() == 1) {
34+
Section section = sections.get(0);
35+
if (section.isExcluded(path)) {
36+
return new ArrayList<>();
37+
}
38+
Entry entry = section.findMatchingEntry(path);
39+
return entry != null ? new ArrayList<>(entry.getOwners()) : null;
40+
}
41+
3642
Set<String> owners = null;
3743
for (Section section : sections) {
38-
if (section.isExcluded(pathCharacters)) {
44+
if (section.isExcluded(path)) {
3945
if (owners == null) {
4046
owners = new LinkedHashSet<>();
4147
}
4248
continue;
4349
}
4450

45-
Entry entry = section.findMatchingEntry(pathCharacters);
51+
Entry entry = section.findMatchingEntry(path);
4652
if (entry != null) {
4753
if (owners == null) {
4854
owners = new LinkedHashSet<>();
@@ -90,36 +96,4 @@ public static Codeowners parse(Reader r) throws IOException {
9096
sections.addAll(namedSections.values());
9197
return new CodeownersImpl(sections);
9298
}
93-
94-
private static final class Section {
95-
96-
private final Deque<Entry> entries = new ArrayDeque<>();
97-
private final Collection<Entry> exclusions = new ArrayList<>();
98-
99-
private void add(Entry entry) {
100-
if (entry.isExclusion()) {
101-
exclusions.add(entry);
102-
} else {
103-
entries.offerFirst(entry);
104-
}
105-
}
106-
107-
private boolean isExcluded(char[] path) {
108-
for (Entry exclusion : exclusions) {
109-
if (exclusion.getMatcher().consume(path, 0) >= 0) {
110-
return true;
111-
}
112-
}
113-
return false;
114-
}
115-
116-
private @Nullable Entry findMatchingEntry(char[] path) {
117-
for (Entry entry : entries) {
118-
if (entry.getMatcher().consume(path, 0) >= 0) {
119-
return entry;
120-
}
121-
}
122-
return null;
123-
}
124-
}
12599
}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/Entry.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
11
package datadog.trace.civisibility.codeowners;
22

33
import datadog.trace.civisibility.codeowners.matcher.Matcher;
4+
import java.util.ArrayList;
45
import java.util.Collection;
6+
import java.util.LinkedHashSet;
7+
import javax.annotation.Nullable;
58

69
public class Entry {
710

811
private final Matcher matcher;
912
private final Collection<String> owners;
1013
private final boolean exclusion;
14+
private final @Nullable String indexKey;
1115

12-
public Entry(Matcher matcher, Collection<String> owners, boolean exclusion) {
16+
public Entry(
17+
Matcher matcher, Collection<String> owners, boolean exclusion, @Nullable String indexKey) {
1318
this.matcher = matcher;
14-
this.owners = owners;
19+
this.owners = owners.size() > 1 ? new ArrayList<>(new LinkedHashSet<>(owners)) : owners;
1520
this.exclusion = exclusion;
21+
this.indexKey = indexKey;
1622
}
1723

1824
public Matcher getMatcher() {
@@ -26,4 +32,8 @@ public Collection<String> getOwners() {
2632
public boolean isExclusion() {
2733
return exclusion;
2834
}
35+
36+
public @Nullable String getIndexKey() {
37+
return indexKey;
38+
}
2939
}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/EntryBuilder.java

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,13 @@ public EntryBuilder(CharacterMatcher.Factory characterMatcherFactory, String s)
7373
offset++;
7474
}
7575

76+
String indexKey = parseIndexKey();
7677
Matcher matcher = parseMatcher();
7778
Collection<String> owners = exclusion ? Collections.emptyList() : parseOwners();
7879
if (!exclusion && owners.isEmpty()) {
7980
owners = sectionDefaultOwners;
8081
}
81-
return new Entry(matcher, owners, exclusion);
82+
return new Entry(matcher, owners, exclusion, indexKey);
8283

8384
} catch (Exception e) {
8485
log.warn("Skipping malformed CODEOWNERS entry: {}", new String(c), e);
@@ -224,6 +225,46 @@ private Matcher parseMatcher() {
224225
return new CompositeMatcher(characterMatchers.toArray(new Matcher[0]));
225226
}
226227

228+
private @Nullable String parseIndexKey() {
229+
// Index by the first two fixed path segments; patterns without a safe prefix stay linear.
230+
int position = offset;
231+
int patternEnd = position;
232+
while (patternEnd < c.length && !isPatternTerminator(c[patternEnd])) {
233+
patternEnd++;
234+
}
235+
// Trailing-slash matchers can match beyond a segment boundary, making their prefix unsafe.
236+
if (patternEnd > position && c[patternEnd - 1] == '/') {
237+
return null;
238+
}
239+
boolean patternContainsSlashes = c[position] == '/';
240+
if (patternContainsSlashes) {
241+
position++;
242+
}
243+
int prefixStart = position;
244+
int firstSeparator = -1;
245+
for (; position < c.length && !isPatternTerminator(c[position]); position++) {
246+
char character = c[position];
247+
if (character == '*' || character == '?' || character == '[') {
248+
return null;
249+
}
250+
if (character == '\\') {
251+
return null;
252+
}
253+
if (character == '/') {
254+
patternContainsSlashes = true;
255+
if (firstSeparator >= 0) {
256+
return new String(c, prefixStart, position - prefixStart);
257+
}
258+
firstSeparator = position;
259+
}
260+
}
261+
262+
if (!patternContainsSlashes || firstSeparator < 0 || firstSeparator == position - 1) {
263+
return null;
264+
}
265+
return new String(c, prefixStart, position - prefixStart);
266+
}
267+
227268
private boolean consumeDoubleAsterisk() {
228269
int position = offset;
229270

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package datadog.trace.civisibility.codeowners;
2+
3+
import java.util.ArrayList;
4+
import java.util.Collections;
5+
import java.util.HashMap;
6+
import java.util.List;
7+
import java.util.Map;
8+
import javax.annotation.Nullable;
9+
10+
/**
11+
* Resolves entries by rule priority, switching large rule sets from linear scans to fixed-prefix
12+
* buckets while retaining a fallback for patterns that cannot be indexed.
13+
*/
14+
final class EntryIndex {
15+
16+
private static final int MIN_INDEX_SIZE = 512;
17+
18+
private List<IndexedEntry> entries = new ArrayList<>();
19+
private Map<String, List<IndexedEntry>> entriesByKey;
20+
private List<IndexedEntry> unindexedEntries;
21+
private int indexableEntryCount;
22+
23+
void add(Entry entry, int order) {
24+
IndexedEntry indexedEntry = new IndexedEntry(entry, order);
25+
if (entriesByKey != null) {
26+
index(indexedEntry);
27+
} else {
28+
entries.add(indexedEntry);
29+
if (entry.getIndexKey() != null) {
30+
indexableEntryCount++;
31+
}
32+
if (entries.size() >= MIN_INDEX_SIZE && indexableEntryCount * 2 >= entries.size()) {
33+
entriesByKey = new HashMap<>();
34+
unindexedEntries = new ArrayList<>();
35+
for (IndexedEntry existingEntry : entries) {
36+
index(existingEntry);
37+
}
38+
entries = Collections.emptyList();
39+
}
40+
}
41+
}
42+
43+
@Nullable
44+
Entry find(String path) {
45+
IndexedEntry entry = findIndexedEntry(path);
46+
return entry != null ? entry.entry : null;
47+
}
48+
49+
private @Nullable IndexedEntry findIndexedEntry(String path) {
50+
if (entriesByKey == null) {
51+
return findFirstMatch(entries, path);
52+
}
53+
IndexedEntry unindexedMatch = findFirstMatch(unindexedEntries, path);
54+
55+
int firstSeparator = path.indexOf('/');
56+
if (firstSeparator < 0) {
57+
return unindexedMatch;
58+
}
59+
int secondSeparator = path.indexOf('/', firstSeparator + 1);
60+
int keyEnd = secondSeparator >= 0 ? secondSeparator : path.length();
61+
List<IndexedEntry> indexedEntries = entriesByKey.get(path.substring(0, keyEnd));
62+
IndexedEntry indexedMatch = findFirstMatch(indexedEntries, path);
63+
64+
if (unindexedMatch == null) {
65+
return indexedMatch;
66+
}
67+
if (indexedMatch == null) {
68+
return unindexedMatch;
69+
}
70+
return indexedMatch.order > unindexedMatch.order ? indexedMatch : unindexedMatch;
71+
}
72+
73+
private void index(IndexedEntry indexedEntry) {
74+
String indexKey = indexedEntry.entry.getIndexKey();
75+
if (indexKey != null) {
76+
entriesByKey.computeIfAbsent(indexKey, key -> new ArrayList<>()).add(indexedEntry);
77+
} else {
78+
unindexedEntries.add(indexedEntry);
79+
}
80+
}
81+
82+
private static @Nullable IndexedEntry findFirstMatch(
83+
@Nullable List<IndexedEntry> entries, String path) {
84+
if (entries != null) {
85+
for (int i = entries.size() - 1; i >= 0; i--) {
86+
IndexedEntry entry = entries.get(i);
87+
if (entry.entry.getMatcher().consume(path, 0) >= 0) {
88+
return entry;
89+
}
90+
}
91+
}
92+
return null;
93+
}
94+
95+
private static final class IndexedEntry {
96+
97+
private final Entry entry;
98+
private final int order;
99+
100+
private IndexedEntry(Entry entry, int order) {
101+
this.entry = entry;
102+
this.order = order;
103+
}
104+
}
105+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package datadog.trace.civisibility.codeowners;
2+
3+
import javax.annotation.Nullable;
4+
5+
/** Groups ownership and exclusion rules for a CODEOWNERS section while preserving rule order. */
6+
final class Section {
7+
8+
private final EntryIndex entries = new EntryIndex();
9+
private final EntryIndex exclusions = new EntryIndex();
10+
private int entryOrder;
11+
12+
void add(Entry entry) {
13+
if (entry.isExclusion()) {
14+
exclusions.add(entry, entryOrder++);
15+
} else {
16+
entries.add(entry, entryOrder++);
17+
}
18+
}
19+
20+
boolean isExcluded(String path) {
21+
return exclusions.find(path) != null;
22+
}
23+
24+
@Nullable
25+
Entry findMatchingEntry(String path) {
26+
return entries.find(path);
27+
}
28+
}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/matcher/AsteriskMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ public class AsteriskMatcher implements Matcher {
77
private AsteriskMatcher() {}
88

99
@Override
10-
public int consume(char[] line, int offset) {
11-
return offset < line.length && line[offset] != '/' ? 1 : -1;
10+
public int consume(String line, int offset) {
11+
return offset < line.length() && line.charAt(offset) != '/' ? 1 : -1;
1212
}
1313

1414
@Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/matcher/CharacterMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ private CharacterMatcher(char character) {
1212
}
1313

1414
@Override
15-
public int consume(char[] line, int offset) {
16-
return offset < line.length && line[offset] == character ? 1 : -1;
15+
public int consume(String line, int offset) {
16+
return offset < line.length() && line.charAt(offset) == character ? 1 : -1;
1717
}
1818

1919
@Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/matcher/CompositeMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ public CompositeMatcher(Matcher[] delegates) {
99
}
1010

1111
@Override
12-
public int consume(char[] line, int offset) {
12+
public int consume(String line, int offset) {
1313
return consume(line, offset, 0);
1414
}
1515

16-
private int consume(char[] line, int offset, int matcherOffset) {
16+
private int consume(String line, int offset, int matcherOffset) {
1717
int position = offset;
1818
while (matcherOffset < delegates.length) {
1919
Matcher delegate = delegates[matcherOffset];

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/matcher/DoubleAsteriskMatcher.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ public class DoubleAsteriskMatcher implements Matcher {
77
private DoubleAsteriskMatcher() {}
88

99
@Override
10-
public int consume(char[] line, int offset) {
11-
if (offset == line.length) {
10+
public int consume(String line, int offset) {
11+
if (offset == line.length()) {
1212
return -1;
1313
}
1414

1515
int position = offset;
16-
while (position < line.length && line[position++] != '/') {}
16+
while (position < line.length() && line.charAt(position++) != '/') {}
1717
return position - offset;
1818
}
1919

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/codeowners/matcher/EndOfLineMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ public class EndOfLineMatcher implements Matcher {
55
public static final Matcher INSTANCE = new EndOfLineMatcher();
66

77
@Override
8-
public int consume(char[] line, int offset) {
9-
return offset == line.length ? 0 : -1;
8+
public int consume(String line, int offset) {
9+
return offset == line.length() ? 0 : -1;
1010
}
1111

1212
@Override

0 commit comments

Comments
 (0)