Improve performance of parentof operator
Make parentof operator use IndexPredicates to improve performance.
On a site with ~20k open changes, this change significantly improves
performance of queries like [1]. They take ~25ms with this change and
~4s without it.
[1] gerrit query parentof:<change> status:new
Release-Notes: skip
Change-Id: I68fc26f8a6b64a3615732d885e0d118fd2752f3d
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index da36633..e36dbfc 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -26,6 +26,7 @@
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.gerrit.entities.Account;
@@ -104,6 +105,8 @@
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
/** Parses a query string meant to be applied to change objects. */
public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuilder> {
@@ -813,11 +816,28 @@
List<ChangeData> changes = parseChangeData(value);
List<Predicate<ChangeData>> or = new ArrayList<>(changes.size());
for (ChangeData c : changes) {
- or.add(new ParentOfPredicate(value, c, args.repoManager));
+ for (RevCommit revCommit : getParents(c)) {
+ or.add(ChangePredicates.commitPrefix(revCommit.getId().getName()));
+ }
}
return Predicate.or(or);
}
+ private Set<RevCommit> getParents(ChangeData change) {
+ PatchSet ps = change.currentPatchSet();
+ try (Repository repo = args.repoManager.openRepository(change.project());
+ RevWalk walk = new RevWalk(repo)) {
+ RevCommit c = walk.parseCommit(ps.commitId());
+ return Sets.newHashSet(c.getParents());
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format(
+ "Loading commit %s for ps %d of change %d failed.",
+ ps.commitId(), ps.id().get(), ps.id().changeId().get()),
+ e);
+ }
+ }
+
@Operator
public Predicate<ChangeData> parentproject(String name) {
return new ParentProjectPredicate(args.projectCache, args.childProjects, name);
diff --git a/java/com/google/gerrit/server/query/change/ParentOfPredicate.java b/java/com/google/gerrit/server/query/change/ParentOfPredicate.java
deleted file mode 100644
index e48d586..0000000
--- a/java/com/google/gerrit/server/query/change/ParentOfPredicate.java
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright (C) 2021 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import com.google.common.collect.Sets;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.index.query.Matchable;
-import com.google.gerrit.index.query.OperatorPredicate;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import java.io.IOException;
-import java.util.Set;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
-
-public class ParentOfPredicate extends OperatorPredicate<ChangeData>
- implements Matchable<ChangeData> {
- protected final Set<RevCommit> parents;
-
- public ParentOfPredicate(String value, ChangeData change, GitRepositoryManager repoManager) {
- super(ChangeQueryBuilder.FIELD_PARENTOF, value);
- this.parents = getParents(change, repoManager);
- }
-
- @Override
- public boolean match(ChangeData changeData) {
- return changeData.patchSets().stream().anyMatch(ps -> parents.contains(ps.commitId()));
- }
-
- @Override
- public int getCost() {
- return 1;
- }
-
- protected Set<RevCommit> getParents(ChangeData change, GitRepositoryManager repoManager) {
- PatchSet ps = change.currentPatchSet();
- try (Repository repo = repoManager.openRepository(change.project());
- RevWalk walk = new RevWalk(repo)) {
- RevCommit c = walk.parseCommit(ps.commitId());
- return Sets.newHashSet(c.getParents());
- } catch (IOException e) {
- throw new StorageException(
- String.format(
- "Loading commit %s for ps %d of change %d failed.",
- ps.commitId(), ps.id().get(), ps.id().changeId().get()),
- e);
- }
- }
-}
