Skip to content

Commit 2819621

Browse files
ceekay47facebook-github-bot
authored andcommitted
Refactor StaticSelector.match to avoid throwing exceptions
Summary: 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 a5e12fd commit 2819621

File tree

1 file changed

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

1 file changed

+28
-11
lines changed

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

Lines changed: 28 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,12 @@ 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+
principalRegex.ifPresent(p -> patternGroupsBuilder.put(p, extractNamedGroups(p)));
92+
this.patternNamedGroups = patternGroupsBuilder.build();
93+
8694
Set<String> unresolvedVariables = Sets.difference(group.getVariableNames(), variableNames);
8795
checkArgument(unresolvedVariables.isEmpty(), "unresolved variables %s in resource group ID '%s', available: %s\"", unresolvedVariables, group, variableNames);
8896
}
@@ -167,19 +175,28 @@ private static void addNamedGroups(Pattern pattern, HashSet<String> variables)
167175
}
168176
}
169177

178+
private static ImmutableSet<String> extractNamedGroups(Pattern pattern)
179+
{
180+
ImmutableSet.Builder<String> groups = ImmutableSet.builder();
181+
Matcher matcher = NAMED_GROUPS_PATTERN.matcher(pattern.pattern());
182+
while (matcher.find()) {
183+
groups.add(matcher.group(1));
184+
}
185+
return groups.build();
186+
}
187+
170188
private void addVariableValues(Pattern pattern, String candidate, Map<String, String> mapping)
171189
{
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
190+
Set<String> groups = patternNamedGroups.getOrDefault(pattern, ImmutableSet.of());
191+
if (groups.isEmpty()) {
192+
return;
193+
}
194+
Matcher keyMatcher = pattern.matcher(candidate);
195+
if (keyMatcher.find()) {
196+
for (String key : groups) {
197+
String value = keyMatcher.group(key);
198+
if (value != null) {
199+
mapping.put(key, value);
183200
}
184201
}
185202
}

0 commit comments

Comments
 (0)