commit 942e8235fb5a1f85279ae4bc909e74d7fa9fade1
author Edwin Kempin <ekempin@google.com> Thu Apr 04 13:42:15 2024 +0000
committer Edwin Kempin <ekempin@google.com> Thu Apr 04 14:36:21 2024 +0000
tree 6f37410ac91b671e35f0b2bb1991a10558edc36d
parent 6a49304e1b53118847a5805d7d498af9dd51ca65

ChangeJson: Don't get all ref states when only meta ref is needed

Loading all ref states can be expensive. When we need the meta ref only
load this one ref and not all ref states.

Release-Notes: skip
Change-Id: I112a862debbc79719cce12085b3a4757fbf6b96e
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/server/change/ChangeJson.java

diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 90b6e4f..f5af631 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -63,7 +63,6 @@
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
 import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.entities.SubmitRecord;
 import com.google.gerrit.entities.SubmitRecord.Status;
 import com.google.gerrit.entities.SubmitRequirementResult;
@@ -86,7 +85,6 @@
 import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
 import com.google.gerrit.extensions.common.TrackingIdInfo;
 import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.index.RefState;
 import com.google.gerrit.index.query.QueryResult;
 import com.google.gerrit.metrics.Description;
 import com.google.gerrit.metrics.Description.Units;
@@ -868,10 +866,7 @@
         TraceContext.newTimer(
             "Get change meta ref",
             Metadata.builder().changeId(cd.change().getId().get()).build())) {
-      String metaName = RefNames.changeMetaRef(cd.getId());
-      Optional<RefState> metaState =
-          cd.getRefStates().values().stream().filter(r -> r.ref().equals(metaName)).findAny();
-      return metaState.map(RefState::id);
+      return cd.metaRevision();
     }
   }
 

java/com/google/gerrit/server/query/change/ChangeData.java

diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 99df1b2..b607691 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -691,10 +691,10 @@
     return this;
   }
 
-  public ObjectId metaRevisionOrThrow() {
+  public Optional<ObjectId> metaRevision() {
     if (notes == null) {
       if (metaRevision != null) {
-        return metaRevision;
+        return Optional.of(metaRevision);
       }
       if (refStates != null) {
         ImmutableSet<RefState> refs = refStates.get(project);
@@ -702,18 +702,25 @@
           String metaRef = RefNames.changeMetaRef(getId());
           for (RefState r : refs) {
             if (r.ref().equals(metaRef)) {
-              return r.id();
+              return Optional.of(r.id());
             }
           }
         }
       }
-      throwIfNotLazyLoad("metaRevision");
+      if (!lazyload()) {
+        return Optional.empty();
+      }
 
       @SuppressWarnings("unused")
       var unused = notes();
     }
     metaRevision = notes.getRevision();
-    return metaRevision;
+    return Optional.of(metaRevision);
+  }
+
+  public ObjectId metaRevisionOrThrow() {
+    return metaRevision()
+        .orElseThrow(() -> new IllegalStateException("'metaRevision' field not populated"));
   }
 
   boolean fastIsVisibleTo(CurrentUser user) {