Optimize output stream encoding. (#1423)

* Use a BufferedWriter which drastically speeds up json encoding, as the
  encoder makes a lot of one-character writes so not buffering them
  leads to significant uft-8 encoding overhead.
* Encode directly into the connection as opposed to an intermediate
  byte array.
* Switch to try-with-resources for better resource safety

Author: vkryachko

transport/transport-backend-cct/src/main/java/com/google/android/datatransport/cct/CctTransportBackend.java

@@ -42,17 +42,16 @@
 import com.google.android.datatransport.runtime.time.Clock;
 import com.google.firebase.encoders.DataEncoder;
 import com.google.firebase.encoders.EncodingException;
-import java.io.ByteArrayOutputStream;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.nio.ByteBuffer;
-import java.nio.channels.Channels;
-import java.nio.channels.WritableByteChannel;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Calendar;
@@ -264,50 +263,46 @@ private HttpResponse doSend(HttpRequest request) throws IOException {
       connection.setRequestProperty(API_KEY_HEADER_KEY, request.apiKey);
     }
 
-    WritableByteChannel channel = Channels.newChannel(connection.getOutputStream());
-    try {
-      ByteArrayOutputStream output = new ByteArrayOutputStream();
-      GZIPOutputStream gzipOutputStream = new GZIPOutputStream(output);
+    try (OutputStream conn = connection.getOutputStream();
+        OutputStream outputStream = new GZIPOutputStream(conn)) {
+      // note: it's very important to use a BufferedWriter for efficient use of resources as the
+      // JsonWriter often writes one character at a time.
+      dataEncoder.encode(
+          request.requestBody, new BufferedWriter(new OutputStreamWriter(outputStream)));
+    } catch (EncodingException | IOException e) {
+      Logging.e(LOG_TAG, "Couldn't encode request, returning with 400", e);
+      return new HttpResponse(400, null, 0);
+    }
 
-      try {
-        dataEncoder.encode(request.requestBody, new OutputStreamWriter(gzipOutputStream));
-      } catch (EncodingException | IOException e) {
-        Logging.e(LOG_TAG, "Couldn't encode request, returning with 400", e);
-        return new HttpResponse(400, null, 0);
-      } finally {
-        gzipOutputStream.close();
-      }
-      channel.write(ByteBuffer.wrap(output.toByteArray()));
-      int responseCode = connection.getResponseCode();
-      Logging.i(LOG_TAG, "Status Code: " + responseCode);
-      Logging.i(LOG_TAG, "Content-Type: " + connection.getHeaderField("Content-Type"));
-      Logging.i(LOG_TAG, "Content-Encoding: " + connection.getHeaderField("Content-Encoding"));
-
-      if (responseCode == 302 || responseCode == 301 || responseCode == 307) {
-        String redirect = connection.getHeaderField("Location");
-        return new HttpResponse(responseCode, new URL(redirect), 0);
-      }
-      if (responseCode != 200) {
-        return new HttpResponse(responseCode, null, 0);
-      }
+    int responseCode = connection.getResponseCode();
+    Logging.i(LOG_TAG, "Status Code: " + responseCode);
+    Logging.i(LOG_TAG, "Content-Type: " + connection.getHeaderField("Content-Type"));
+    Logging.i(LOG_TAG, "Content-Encoding: " + connection.getHeaderField("Content-Encoding"));
 
-      InputStream inputStream;
-      String contentEncoding = connection.getHeaderField(CONTENT_ENCODING_HEADER_KEY);
-      if (contentEncoding != null && contentEncoding.equals(GZIP_CONTENT_ENCODING)) {
-        inputStream = new GZIPInputStream(connection.getInputStream());
-      } else {
-        inputStream = connection.getInputStream();
-      }
-      try {
-        long nextRequestMillis =
-            LogResponse.fromJson(new InputStreamReader(inputStream)).getNextRequestWaitMillis();
-        return new HttpResponse(responseCode, null, nextRequestMillis);
-      } finally {
-        inputStream.close();
-      }
-    } finally {
-      channel.close();
+    if (responseCode == 302 || responseCode == 301 || responseCode == 307) {
+      String redirect = connection.getHeaderField("Location");
+      return new HttpResponse(responseCode, new URL(redirect), 0);
+    }
+    if (responseCode != 200) {
+      return new HttpResponse(responseCode, null, 0);
+    }
+
+    try (InputStream connStream = connection.getInputStream();
+        InputStream inputStream =
+            maybeUnGzip(connStream, connection.getHeaderField(CONTENT_ENCODING_HEADER_KEY))) {
+      long nextRequestMillis =
+          LogResponse.fromJson(new BufferedReader(new InputStreamReader(inputStream)))
+              .getNextRequestWaitMillis();
+      return new HttpResponse(responseCode, null, nextRequestMillis);
+    }
+  }
+
+  private static InputStream maybeUnGzip(InputStream input, String contentEncoding)
+      throws IOException {
+    if (GZIP_CONTENT_ENCODING.equals(contentEncoding)) {
+      return new GZIPInputStream(input);
     }
+    return input;
   }
 
   @Override

transport/transport-backend-cct/src/main/java/com/google/android/datatransport/cct/internal/LogResponse.java

@@ -17,7 +17,6 @@
 import android.util.JsonReader;
 import android.util.JsonToken;
 import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
 import com.google.auto.value.AutoValue;
 import java.io.IOException;
 import java.io.Reader;
@@ -33,7 +32,7 @@ static LogResponse create(long nextRequestWaitMillis) {
     return new AutoValue_LogResponse(nextRequestWaitMillis);
   }
 
-  @Nullable
+  @NonNull
   public static LogResponse fromJson(@NonNull Reader reader) throws IOException {
     JsonReader jsonReader = new JsonReader(reader);
     try {

transport/transport-backend-cct/transport-backend-cct.gradle

@@ -66,21 +66,10 @@ dependencies {
     testImplementation 'junit:junit:4.12'
     testImplementation "com.google.truth:truth:$googleTruthVersion"
     testImplementation 'com.google.truth.extensions:truth-proto-extension:1.0'
-    testImplementation("com.github.tomakehurst:wiremock-jre8:2.21.0") {
-        //Allows us to use the Android version of Apache httpclient instead
-        exclude group: 'org.apache.httpcomponents', module: 'httpclient'
-
-        //Resolves the Duplicate Class Exception
-        //duplicate entry: org/objectweb/asm/AnnotationVisitor.class
-        exclude group: 'asm', module: 'asm'
-
-        //Fixes Warning conflict with Android's version of org.json
-        //org.json:json:20090211 is ignored for debugAndroidTest as it may be conflicting with the internal version provided by Android.
-        exclude group: 'org.json', module: 'json'
-    }
+    testImplementation 'com.github.tomakehurst:wiremock:2.26.3'
     //Android compatible version of Apache httpclient.
     testImplementation 'org.apache.httpcomponents:httpclient-android:4.3.5.1'
-    testImplementation 'org.robolectric:robolectric:4.2'
+    testImplementation 'org.robolectric:robolectric:4.3.1'
     testImplementation 'junit:junit:4.13-beta-2'
 
     androidTestImplementation 'androidx.test:runner:1.2.0'