Skip to content

Commit 3ed6a17

Browse files
ceekay47meta-codesync[bot]
authored andcommitted
Refactor StaticSelector.match to avoid throwing exceptions (#27159)
Summary: Pull Request resolved: #27159 The `addVariableValues` method in `StaticSelector` previously iterated over all `variableNames` and caught `IllegalArgumentException` for each name not present as a named group in the pattern. This is inefficient and relies on exception handling for control flow. This change pre-computes the named groups for each regex pattern at construction time into an `ImmutableMap<Pattern, ImmutableSet<String>>`. The `addVariableValues` method now looks up the pre-computed groups for the given pattern and only iterates over matching group names, avoiding the need to catch exceptions entirely. Differential Revision: D93437091
1 parent 920353f commit 3ed6a17

File tree

1 file changed

+27
-11
lines changed
  • presto-resource-group-managers/src/main/java/com/facebook/presto/resourceGroups

1 file changed

+27
-11
lines changed

presto-resource-group-managers/src/main/java/com/facebook/presto/resourceGroups/StaticSelector.java

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.facebook.presto.spi.resourceGroups.SelectionCriteria;
1919
import com.google.common.annotations.VisibleForTesting;
2020
import com.google.common.collect.ImmutableList;
21+
import com.google.common.collect.ImmutableMap;
2122
import com.google.common.collect.ImmutableSet;
2223
import com.google.common.collect.Sets;
2324

@@ -55,6 +56,7 @@ public class StaticSelector
5556
private final Optional<Pattern> principalRegex;
5657
private final ResourceGroupIdTemplate group;
5758
private final Set<String> variableNames;
59+
private final ImmutableMap<Pattern, ImmutableSet<String>> patternNamedGroups;
5860

5961
public StaticSelector(
6062
Optional<Pattern> userRegex,
@@ -83,6 +85,11 @@ public StaticSelector(
8385
sourceRegex.ifPresent(s -> addNamedGroups(s, variableNames));
8486
this.variableNames = ImmutableSet.copyOf(variableNames);
8587

88+
ImmutableMap.Builder<Pattern, ImmutableSet<String>> patternGroupsBuilder = ImmutableMap.builder();
89+
userRegex.ifPresent(u -> patternGroupsBuilder.put(u, extractNamedGroups(u)));
90+
sourceRegex.ifPresent(s -> patternGroupsBuilder.put(s, extractNamedGroups(s)));
91+
this.patternNamedGroups = patternGroupsBuilder.build();
92+
8693
Set<String> unresolvedVariables = Sets.difference(group.getVariableNames(), variableNames);
8794
checkArgument(unresolvedVariables.isEmpty(), "unresolved variables %s in resource group ID '%s', available: %s\"", unresolvedVariables, group, variableNames);
8895
}
@@ -167,19 +174,28 @@ private static void addNamedGroups(Pattern pattern, HashSet<String> variables)
167174
}
168175
}
169176

177+
private static ImmutableSet<String> extractNamedGroups(Pattern pattern)
178+
{
179+
ImmutableSet.Builder<String> groups = ImmutableSet.builder();
180+
Matcher matcher = NAMED_GROUPS_PATTERN.matcher(pattern.pattern());
181+
while (matcher.find()) {
182+
groups.add(matcher.group(1));
183+
}
184+
return groups.build();
185+
}
186+
170187
private void addVariableValues(Pattern pattern, String candidate, Map<String, String> mapping)
171188
{
172-
for (String key : variableNames) {
173-
Matcher keyMatcher = pattern.matcher(candidate);
174-
if (keyMatcher.find()) {
175-
try {
176-
String value = keyMatcher.group(key);
177-
if (value != null) {
178-
mapping.put(key, value);
179-
}
180-
}
181-
catch (IllegalArgumentException ignored) {
182-
// there was no capturing group with the specified name
189+
Set<String> groups = patternNamedGroups.getOrDefault(pattern, ImmutableSet.of());
190+
if (groups.isEmpty()) {
191+
return;
192+
}
193+
Matcher keyMatcher = pattern.matcher(candidate);
194+
if (keyMatcher.find()) {
195+
for (String key : groups) {
196+
String value = keyMatcher.group(key);
197+
if (value != null) {
198+
mapping.put(key, value);
183199
}
184200
}
185201
}

0 commit comments

Comments
 (0)