summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNasser Grainawi <nasser.grainawi@linaro.org>2022-08-22 14:15:19 -0600
committerKaushik Lingarkar <kaushik.lingarkar@linaro.org>2022-08-25 14:23:40 -0700
commit0e93cd75fd9dc8a4398e4bc5740e405a0504ad22 (patch)
tree5338480fb2b2045a20b40f8497a9afcc9c8817df
parent32a7f5fea30a2f7c4a60392ab5b0be63f7825e07 (diff)
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
-rw-r--r--java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java21
1 files changed, 9 insertions, 12 deletions
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index a66c43ae4a..bb10de5789 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 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
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 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
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 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
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) {