opendev/gerrit
Code Issues Proposed changes

Cache RefPattern.shortestExample

Browse Source 
Profiling shows access control heavy servers can spend as much as
1.59% of their CPU time just inside RefPattern.shortestExample. The
parsing of the RegExp with dk.brics.automaton and formatting an
example is not free.

Cache up to 4000 used unique regex patterns to attempt to save CPU
time during access control evaluations.  4000 was chosen as a wild
guess, but it turns out to be 20x larger than the number of distinct
patterns known across the major servers deployed within Google.

Configuring the cache size is more challenging, as the method is a
static class method which cannot get the cache by injection. Hence
we just hardcode the limit 20x larger than the largest set of unique
patterns I have available.

Change-Id: I15601cf332503c5bf8a28d99f6f53f8a0f652453
This commit is contained in:
Shawn Pearce
2016-06-18 14:15:14 -07:00
parent fd0ba728e5
commit 4c07d8c060
1 changed files with 31 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
View File

@@ -14,6 +14,10 @@
package com.google.gerrit.server.project;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.RefConfigSection;
import com.google.gerrit.common.errors.InvalidNameException;
@@ -22,6 +26,7 @@ import dk.brics.automaton.RegExp;
import org.eclipse.jgit.lib.Repository;
import java.util.concurrent.ExecutionException;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -29,15 +34,24 @@ public class RefPattern {
public static final String USERID_SHARDED = "shardeduserid";
public static final String USERNAME = "username";
private static final LoadingCache<String, String> exampleCache = CacheBuilder
.newBuilder()
.maximumSize(4000)
.build(new CacheLoader<String, String>() {
@Override
public String load(String refPattern) {
return example(refPattern);
}
});
public static String shortestExample(String refPattern) {
if (isRE(refPattern)) {
// Since Brics will substitute dot [.] with \0 when generating
// shortest example, any usage of dot will fail in
// Repository.isValidRefName() if not combined with star [*].
// To get around this, we substitute the \0 with an arbitrary
// accepted character.
return toRegExp(refPattern).toAutomaton().getShortestExample(true)
.replace('\0', '-');
try {
return exampleCache.get(refPattern);
} catch (ExecutionException e) {
Throwables.propagateIfPossible(e.getCause());
throw new RuntimeException(e);
}
} else if (refPattern.endsWith("/*")) {
return refPattern.substring(0, refPattern.length() - 1) + '1';
} else {
@@ -45,6 +59,16 @@ public class RefPattern {
}
}
static String example(String refPattern) {
// Since Brics will substitute dot [.] with \0 when generating
// shortest example, any usage of dot will fail in
// Repository.isValidRefName() if not combined with star [*].
// To get around this, we substitute the \0 with an arbitrary
// accepted character.
return toRegExp(refPattern).toAutomaton().getShortestExample(true)
.replace('\0', '-');
}
public static boolean isRE(String refPattern) {
return refPattern.startsWith(AccessSection.REGEX_PREFIX);
}