From 0e93cd75fd9dc8a4398e4bc5740e405a0504ad22 Mon Sep 17 00:00:00 2001
From: Nasser Grainawi <nasser.grainawi@linaro.org>
Date: Mon, 22 Aug 2022 14:15:19 -0600
Subject: 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
---
 .../query/change/ChangeIsVisibleToPredicate.java    | 21 +++++++++------------
 1 file 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) {
-- 
cgit v1.2.3