From 5bcf15cc1e45f01fd0b96dafcd191d4b00764277 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 18 Mar 2015 14:29:28 -0700
Subject: [PATCH] Presize TemporaryBuffer.Heap to a reasonable size

JGit's TemporaryBuffer.Heap recently gained an estimatedSize argument
used for presizing the backing block pointer list. Without this
argument, it assumes callers are going to use the entire maximum size,
and allocates an array large enough to hold enough pointers to
8192-byte blocks to fill the maximum size.

This is pathologically wasteful in RestApiServlet, where we might
allocate one or more buffers with size Integer.MAX_VALUE just to store
JSON serialization results. This allocates an array with over 250k
elements, taking about 2M of memory which is immediately made garbage.
On *every* request.

Most JSON responses are small. We can even be pretty liberal and guess
that they are under 10 blocks (80 KiB) in size, costing us only 10
object pointers (<100 bytes) worth of memory.

Change-Id: I9e1806737706ab2a24693455d64caa7d1067af35
---
 .../gerrit/httpd/restapi/RestApiServlet.java  | 21 ++++++++++++-------
 .../gerrit/server/mail/ChangeEmail.java       |  5 ++++-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 6f4cc08de3..a4a6d39d02 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -142,6 +142,8 @@ public class RestApiServlet extends HttpServlet {
   private static final String JSON_TYPE = "application/json";
   private static final String FORM_TYPE = "application/x-www-form-urlencoded";
 
+  private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
+
   /**
    * Garbage prefix inserted before JSON output to prevent XSSI.
    * <p>
@@ -656,7 +658,7 @@ public class RestApiServlet extends HttpServlet {
       Multimap<String, String> config,
       Object result)
       throws IOException {
-    TemporaryBuffer.Heap buf = heap(Integer.MAX_VALUE);
+    TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE);
     buf.write(JSON_MAGIC);
     Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8));
     Gson gson = newGson(config, req);
@@ -781,7 +783,7 @@ public class RestApiServlet extends HttpServlet {
 
   private static BinaryResult stackJsonString(HttpServletResponse res,
       final BinaryResult src) throws IOException {
-    TemporaryBuffer.Heap buf = heap(Integer.MAX_VALUE);
+    TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE);
     buf.write(JSON_MAGIC);
     try(Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8));
         JsonWriter json = new JsonWriter(w)) {
@@ -1053,10 +1055,15 @@ public class RestApiServlet extends HttpServlet {
     return false;
   }
 
+  private static int base64MaxSize(long n) {
+    return 4 * IntMath.divide((int) n, 3, CEILING);
+  }
+
   private static BinaryResult base64(BinaryResult bin)
       throws IOException {
-    int max = 4 * IntMath.divide((int) bin.getContentLength(), 3, CEILING);
-    TemporaryBuffer.Heap buf = heap(max);
+    int maxSize = base64MaxSize(bin.getContentLength());
+    int estSize = Math.min(base64MaxSize(HEAP_EST_SIZE), maxSize);
+    TemporaryBuffer.Heap buf = heap(estSize, maxSize);
     OutputStream encoded = BaseEncoding.base64().encodingStream(
         new OutputStreamWriter(buf, ISO_8859_1));
     bin.writeTo(encoded);
@@ -1066,7 +1073,7 @@ public class RestApiServlet extends HttpServlet {
 
   private static BinaryResult compress(BinaryResult bin)
       throws IOException {
-    TemporaryBuffer.Heap buf = heap(20 << 20);
+    TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, 20 << 20);
     GZIPOutputStream gz = new GZIPOutputStream(buf);
     bin.writeTo(gz);
     gz.close();
@@ -1083,8 +1090,8 @@ public class RestApiServlet extends HttpServlet {
     }.setContentLength(buf.length());
   }
 
-  private static Heap heap(int max) {
-    return new TemporaryBuffer.Heap(max);
+  private static Heap heap(int est, int max) {
+    return new TemporaryBuffer.Heap(est, max);
   }
 
   @SuppressWarnings("serial")
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index ac2345565a..5642ec1598 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -379,6 +379,8 @@ public abstract class ChangeEmail extends NotificationEmail {
     return args.settings.includeDiff;
   }
 
+  private static int HEAP_EST_SIZE = 32 * 1024;
+
   /** Show patch set as unified difference. */
   public String getUnifiedDiff() {
     PatchList patchList;
@@ -394,8 +396,9 @@ public abstract class ChangeEmail extends NotificationEmail {
       return "";
     }
 
+    int maxSize = args.settings.maximumDiffSize;
     TemporaryBuffer.Heap buf =
-        new TemporaryBuffer.Heap(args.settings.maximumDiffSize);
+        new TemporaryBuffer.Heap(Math.min(HEAP_EST_SIZE, maxSize), maxSize);
     try (DiffFormatter fmt = new DiffFormatter(buf)) {
       Repository git;
       try {