ChangeIsVisibleToPredicate: Only create PermissionBackend.WithUser once
No need to repeat this work for every change we're trying to match and
unless the `user` is the current user, we'll be creating a
new IdentifiedUser each time which adds an overhead that degrades
query performance.
On a site with around 20k open changes a query where the visibility is
checked for an account that cannot see most/all changes improves from
~14s to ~9s.
Change-Id: I9e797fcb3622ed1b0d8313d843c95f7dfaf490f5
Release-Notes: Improve performance of queries that check the visibility of changes wrt a non-current user
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index a66c43ae..bb10de5 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -44,9 +44,8 @@
protected final ChangeNotes.Factory notesFactory;
protected final CurrentUser user;
- protected final PermissionBackend permissionBackend;
protected final ProjectCache projectCache;
- private final Provider<AnonymousUser> anonymousUserProvider;
+ private final PermissionBackend.WithUser withUser;
@Inject
public ChangeIsVisibleToPredicate(
@@ -58,9 +57,14 @@
super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user));
this.notesFactory = notesFactory;
this.user = user;
- this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
- this.anonymousUserProvider = anonymousUserProvider;
+ withUser =
+ user.isIdentifiedUser()
+ ? permissionBackend.absentUser(user.getAccountId())
+ : permissionBackend.user(
+ Optional.of(user)
+ .filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
+ .orElseGet(anonymousUserProvider::get));
}
@Override
@@ -79,17 +83,10 @@
return false;
}
if (!projectState.get().statePermitsRead()) {
- logger.atFine().log("Filter out change %s of non-reabable project %s", cd, cd.project());
+ logger.atFine().log("Filter out change %s of non-readable project %s", cd, cd.project());
return false;
}
- PermissionBackend.WithUser withUser =
- user.isIdentifiedUser()
- ? permissionBackend.absentUser(user.getAccountId())
- : permissionBackend.user(
- Optional.of(user)
- .filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
- .orElseGet(anonymousUserProvider::get));
try {
withUser.change(cd).check(ChangePermission.READ);
} catch (PermissionBackendException e) {
