opendev/gerrit
Code Issues Proposed changes

Add permission_sort cache to remember sort orderings

Browse Source 
Running the MostSpecificComparator on each access control request
to determine the order of AccessSections is expensive, especially
when regular expressions are used in sections and must be converted
to Automatons to evaulate their distance from the target.

Cache the order sections should be sorted in, making any future
sorting for the same reference name and same set of section patterns
cheaper.  Since the sorting is not project specific, the cache can
be more general and cover a lot of the server if most sections are
defined by parent projects.

We don't need to worry about flushing this cache, as it only sorts
the sort order, and the original input order is part of the cache
element key.  Any changes to access controls will still be reflected
in the result, and a change in inheritance or addition/removal
of sections will cause a new cache key to be used, discarding the
prior one via LRU when the cache is full.

Change-Id: Ied06561c9124f6ba8f5ea857a0eb17f47db2bc23
This commit is contained in:
Shawn O. Pearce
2011-06-24 11:01:25 -07:00
parent a429b1a2db
commit 0c1abdb2b7
7 changed files with 430 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.cache.ConcurrentHashMapCache;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -211,6 +212,15 @@ public class RefControlTest extends TestCase {
assertTrue("d can read", d.controlForRef("refs/sb/dev/heads/foobar").isVisible());
}
public void testSortWithRegex() {
grant(local, READ, devs, "^refs/heads/.*");
grant(parent, READ, anonymous, "^refs/heads/.*-QA-.*");
ProjectControl u = user(devs), d = user(devs);
assertTrue("u can read", u.controlForRef("refs/heads/foo-QA-bar").isVisible());
assertTrue("d can read", d.controlForRef("refs/heads/foo-QA-bar").isVisible());
}
// -----------------------------------------------------------------------
private final Map<Project.NameKey, ProjectState> all;
@@ -219,6 +229,8 @@ public class RefControlTest extends TestCase {
private ProjectConfig local;
private ProjectConfig parent;
private PermissionCollection.Factory sectionSorter;
private final AccountGroup.UUID admin = new AccountGroup.UUID("test.admin");
private final AccountGroup.UUID anonymous = AccountGroup.ANONYMOUS_USERS;
private final AccountGroup.UUID registered = AccountGroup.REGISTERED_USERS;
@@ -288,6 +300,11 @@ public class RefControlTest extends TestCase {
local = new ProjectConfig(new Project.NameKey("local"));
local.createInMemory();
local.getProject().setParentName(parent.getProject().getName());
sectionSorter =
new PermissionCollection.Factory(
new SectionSortCache(
new ConcurrentHashMapCache<SectionSortCache.EntryKey, SectionSortCache.EntryVal>()));
}
private static void assertOwner(String ref, ProjectControl u) {
@@ -332,6 +349,7 @@ public class RefControlTest extends TestCase {
return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
Collections.<AccountGroup.UUID> emptySet(), schema, groupCache,
sectionSorter,
canonicalWebUrl, new MockUser(name, memberOf),
newProjectState());
}