From 4cd05b2c5ec2f88bd6d93620356510b6574f04fc Mon Sep 17 00:00:00 2001
From: Shawn Pearce <sop@google.com>
Date: Sat, 17 Sep 2016 22:45:33 -0700
Subject: [PATCH] Only compute submittable during submitted_together

The submittable field of ChangeInfo is undocumented and only used
by the Submitted Together tab of the web UI. It's also quite costly
to compute as the logic currently runs the Prolog rules in the
default "accurate" mode, ignoring the setFastEvalLabels(true) mode
used by DETAILED_LABELS.

Computing submittable for every search result is one of the reasons
dashboards are slow on gerrit-review. Each change is run through the
full set of rules, which requires group membership checks for every
single reviewer vote. These membership lookups take long enough that
the sequential evaluation adds up to a noticable delay.

Just disable the field, except during the submitted_together API.

Change-Id: Iffb893f31c5c01306ace4fb13a43f5f91dd9a2da
---
 Documentation/rest-api-changes.txt                   |  3 +++
 .../acceptance/rest/change/AbstractSubmit.java       |  3 ---
 .../com/google/gerrit/server/change/ChangeJson.java  | 12 ++++++++++--
 .../gerrit/server/change/SubmittedTogether.java      |  1 +
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 59c63cdc01..e28332c017 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -4474,6 +4474,9 @@ Not set for merged changes.
 |`mergeable`          |optional|
 Whether the change is mergeable. +
 Not set for merged changes, or if the change has not yet been tested.
+|`submittable`        |optional|
+Whether the change has been approved by the project submit rules. +
+Provided only by link:#submitted-together[submitted_together].
 |`insertions`         ||
 Number of inserted lines.
 |`deletions`          ||
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 49c248ff1e..a8e33ba68e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -481,9 +481,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
   }
 
   protected void assertSubmittable(String changeId) throws Exception {
-    assertThat(gApi.changes().id(changeId).info().submittable)
-        .named("submit bit on ChangeInfo")
-        .isEqualTo(true);
     RevisionResource rsrc = parseCurrentRevisionResource(changeId);
     UiAction.Description desc = submitHandler.getDescription(rsrc);
     assertThat(desc.isVisible()).named("visible bit on submit action").isTrue();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index e3a56b6870..92965f8fb0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -169,6 +169,7 @@ public class ChangeJson {
   private final ChangeKindCache changeKindCache;
 
   private AccountLoader accountLoader;
+  private boolean includeSubmittable;
   private Map<Change.Id, List<SubmitRecord>> submitRecords;
   private FixInput fix;
 
@@ -222,6 +223,11 @@ public class ChangeJson {
         : EnumSet.copyOf(options);
   }
 
+  public ChangeJson includeSubmittable(boolean include) {
+    includeSubmittable = include;
+    return this;
+  }
+
   public ChangeJson fix(FixInput fix) {
     this.fix = fix;
     return this;
@@ -421,14 +427,16 @@ public class ChangeJson {
     out.topic = in.getTopic();
     out.hashtags = cd.hashtags();
     out.changeId = in.getKey().get();
-    if (in.getStatus() != Change.Status.MERGED) {
+    if (in.getStatus().isOpen()) {
       SubmitTypeRecord str = cd.submitTypeRecord();
       if (str.isOk()) {
         out.submitType = str.type;
       }
       out.mergeable = cd.isMergeable();
+      if (includeSubmittable) {
+        out.submittable = Submit.submittable(cd);
+      }
     }
-    out.submittable = Submit.submittable(cd);
     Optional<ChangedLines> changedLines = cd.changedLines();
     if (changedLines.isPresent()) {
       out.insertions = changedLines.get().insertions;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
index 1afe9602dc..351ebf974d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
@@ -126,6 +126,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
       info.changes = json.create(EnumSet.of(
           ListChangesOption.CURRENT_REVISION,
           ListChangesOption.CURRENT_COMMIT))
+        .includeSubmittable(true)
         .formatChangeDatas(cds);
       info.nonVisibleChanges = hidden;
       return info;