Repository: GoogleCloudPlatform/pgadapter
Pull request #384 (2022-10-10 13:15:28)

perf: write converted values directly to a byte buffer
Write the converted values directly to a byte buffer that can be reused for multiple rows. This reduces the amount of objects that are created and must be garbage collected for each result set that is returned by PGAdapter.

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#384](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (9e9ec7a) into [no-parser-construction](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/ac2dd1f70f6b04bf054f45505d0fe5d12e58e85d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (ac2dd1f) will **increase** coverage by `0.02%`.
> The diff coverage is `97.29%`.

```diff
@@                     Coverage Diff                      @@
##             no-parser-construction     #384      +/-   ##
============================================================
+ Coverage                     86.15%   86.18%   +0.02%     
- Complexity                     1876     1880       +4     
============================================================
  Files                           121      121              
  Lines                          6112     6125      +13     
  Branches                        837      835       -2     
============================================================
+ Hits                           5266     5279      +13     
  Misses                          605      605              
  Partials                        241      241              
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `86.18% <97.29%> (+0.02%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...oogle/cloud/spanner/pgadapter/utils/Converter.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3V0aWxzL0NvbnZlcnRlci5qYXZh) | `89.74% <95.83%> (+9.74%)` | :arrow_up: |
| [.../spanner/pgadapter/statements/CopyToStatement.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvQ29weVRvU3RhdGVtZW50LmphdmE=) | `95.65% <100.00%> (ø)` | |
| [...er/pgadapter/statements/IntermediateStatement.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvSW50ZXJtZWRpYXRlU3RhdGVtZW50LmphdmE=) | `94.44% <100.00%> (+1.11%)` | :arrow_up: |
| [.../spanner/pgadapter/wireoutput/DataRowResponse.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVvdXRwdXQvRGF0YVJvd1Jlc3BvbnNlLmphdmE=) | `81.81% <100.00%> (-10.49%)` | :arrow_down: |
| [...spanner/pgadapter/wireprotocol/ControlMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9Db250cm9sTWVzc2FnZS5qYXZh) | `86.89% <100.00%> (+0.25%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform)


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatement.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatement.java
index bbdce312..38f59b19 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatement.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatement.java
@@ -20,13 +20,13 @@ import com.google.cloud.spanner.Statement;
 import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
 import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
 import com.google.cloud.spanner.pgadapter.ConnectionHandler;
-import com.google.cloud.spanner.pgadapter.ConnectionHandler.QueryMode;
 import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
 import com.google.cloud.spanner.pgadapter.metadata.DescribePortalMetadata;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
 import com.google.cloud.spanner.pgadapter.parsers.Parser;
 import com.google.cloud.spanner.pgadapter.parsers.copy.CopyTreeParser.CopyOptions;
 import com.google.cloud.spanner.pgadapter.parsers.copy.CopyTreeParser.CopyOptions.Format;
+import com.google.cloud.spanner.pgadapter.utils.Converter;
 import com.google.cloud.spanner.pgadapter.wireoutput.CopyDataResponse;
 import com.google.cloud.spanner.pgadapter.wireoutput.CopyDoneResponse;
 import com.google.cloud.spanner.pgadapter.wireoutput.CopyOutResponse;
@@ -161,10 +161,10 @@ public class CopyToStatement extends IntermediatePortalStatement {
   }
 
   @Override
-  public CopyDataResponse createDataRowResponse(ResultSet resultSet, QueryMode mode) {
+  public CopyDataResponse createDataRowResponse(Converter converter) {
     return copyOptions.getFormat() == Format.BINARY
-        ? createBinaryDataResponse(resultSet)
-        : createDataResponse(resultSet);
+        ? createBinaryDataResponse(converter.getResultSet())
+        : createDataResponse(converter.getResultSet());
   }
 
   @Override
@@ -200,6 +200,7 @@ public class CopyToStatement extends IntermediatePortalStatement {
       if (!resultSet.isNull(col)) {
         Parser<?> parser = Parser.create(resultSet, resultSet.getColumnType(col), col);
         data[col] = parser.parse(DataFormat.POSTGRESQL_BINARY);
+
         if (data[col] != null) {
           length += data[col].length;
         }
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/utils/Converter.java b/src/main/java/com/google/cloud/spanner/pgadapter/utils/Converter.java
index d18ba9c5..63feb2fb 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/utils/Converter.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/utils/Converter.java
@@ -16,7 +16,9 @@ package com.google.cloud.spanner.pgadapter.utils;
 
 import com.google.cloud.spanner.ResultSet;
 import com.google.cloud.spanner.Type;
+import com.google.cloud.spanner.pgadapter.ConnectionHandler.QueryMode;
 import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
+import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
 import com.google.cloud.spanner.pgadapter.parsers.ArrayParser;
 import com.google.cloud.spanner.pgadapter.parsers.BinaryParser;
 import com.google.cloud.spanner.pgadapter.parsers.BooleanParser;
@@ -27,11 +29,58 @@ import com.google.cloud.spanner.pgadapter.parsers.LongParser;
 import com.google.cloud.spanner.pgadapter.parsers.NumericParser;
 import com.google.cloud.spanner.pgadapter.parsers.StringParser;
 import com.google.cloud.spanner.pgadapter.parsers.TimestampParser;
+import com.google.cloud.spanner.pgadapter.statements.IntermediateStatement;
 import com.google.common.base.Preconditions;
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
 
 /** Utility class for converting between generic PostgreSQL conversions. */
 public class Converter {
-  private Converter() {}
+  private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(256);
+  private final DataOutputStream outputStream = new DataOutputStream(buffer);
+  private final IntermediateStatement statement;
+  private final QueryMode mode;
+  private final OptionsMetadata options;
+  private final ResultSet resultSet;
+
+  public Converter(
+      IntermediateStatement statement,
+      QueryMode mode,
+      OptionsMetadata options,
+      ResultSet resultSet) {
+    this.statement = statement;
+    this.mode = mode;
+    this.options = options;
+    this.resultSet = resultSet;
+  }
+
+  public ResultSet getResultSet() {
+    return resultSet;
+  }
+
+  public int convertResultSetRowToDataRowResponse() throws IOException {
+    buffer.reset();
+    outputStream.writeShort(resultSet.getColumnCount());
+    for (int column_index = 0; /* column indices start at 0 */
+        column_index < resultSet.getColumnCount();
+        column_index++) {
+      if (resultSet.isNull(column_index)) {
+        outputStream.writeInt(-1);
+      } else {
+        DataFormat format = DataFormat.getDataFormat(column_index, statement, mode, options);
+        byte[] column = Converter.convertToPG(this.resultSet, column_index, format);
+        outputStream.writeInt(column.length);
+        outputStream.write(column);
+      }
+    }
+    return buffer.size();
+  }
+
+  public void writeBuffer(DataOutputStream outputStream) throws IOException {
+    buffer.writeTo(outputStream);
+  }
+
   /**
    * Return the data of the specified column of the {@link ResultSet} as a byte array. The column
    * may not contain a null value.
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/wireoutput/DataRowResponse.java b/src/main/java/com/google/cloud/spanner/pgadapter/wireoutput/DataRowResponse.java
index 8eca0993..4f2fc5ab 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/wireoutput/DataRowResponse.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/wireoutput/DataRowResponse.java
@@ -15,64 +15,29 @@
 package com.google.cloud.spanner.pgadapter.wireoutput;
 
 import com.google.api.core.InternalApi;
-import com.google.cloud.spanner.ResultSet;
-import com.google.cloud.spanner.pgadapter.ConnectionHandler.QueryMode;
-import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
-import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
-import com.google.cloud.spanner.pgadapter.statements.IntermediateStatement;
 import com.google.cloud.spanner.pgadapter.utils.Converter;
 import java.io.DataOutputStream;
-import java.util.ArrayList;
-import java.util.List;
 
 /** Sends to the client specific row contents. */
 @InternalApi
 public class DataRowResponse extends WireOutput {
-
   private static final int HEADER_LENGTH = 4;
-  private static final int COLUMN_NUMBER_LENGTH = 2;
-  private static final int COLUMN_SIZE_LENGTH = 4;
 
-  private final ResultSet resultSet;
-  private final List<byte[]> columns;
+  private final Converter converter;
 
-  public DataRowResponse(
-      DataOutputStream output,
-      IntermediateStatement statement,
-      ResultSet resultSet,
-      OptionsMetadata options,
-      QueryMode mode) {
+  public DataRowResponse(DataOutputStream output, Converter converter) {
     super(output, 0);
-    this.resultSet = resultSet;
-    this.columns = new ArrayList<>(this.resultSet.getColumnCount());
-    this.length = HEADER_LENGTH + COLUMN_NUMBER_LENGTH;
-    // TODO profile this with immense rows/column count to see if it's an aread of optimization.
-    for (int column_index = 0; /* column indices start at 0 */
-        column_index < this.resultSet.getColumnCount();
-        column_index++) {
-      length += COLUMN_SIZE_LENGTH;
-      if (resultSet.isNull(column_index)) {
-        columns.add(null);
-      } else {
-        DataFormat format = DataFormat.getDataFormat(column_index, statement, mode, options);
-        byte[] column = Converter.convertToPG(this.resultSet, column_index, format);
-        length += column.length;
-        columns.add(column);
-      }
-    }
+    this.converter = converter;
+  }
+
+  public void send(boolean flush) throws Exception {
+    this.length = HEADER_LENGTH + converter.convertResultSetRowToDataRowResponse();
+    super.send(flush);
   }
 
   @Override
   protected void sendPayload() throws Exception {
-    this.outputStream.writeShort(this.resultSet.getColumnCount());
-    for (byte[] column_value : columns) {
-      if (column_value == null) {
-        this.outputStream.writeInt(-1);
-      } else {
-        this.outputStream.writeInt(column_value.length);
-        this.outputStream.write(column_value);
-      }
-    }
+    converter.writeBuffer(this.outputStream);
   }
 
   @Override
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/wireprotocol/ControlMessage.java b/src/main/java/com/google/cloud/spanner/pgadapter/wireprotocol/ControlMessage.java
index 354c3110..fb71b2c9 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/wireprotocol/ControlMessage.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/wireprotocol/ControlMessage.java
@@ -38,6 +38,7 @@ import com.google.cloud.spanner.pgadapter.error.Severity;
 import com.google.cloud.spanner.pgadapter.metadata.SendResultSetState;
 import com.google.cloud.spanner.pgadapter.statements.BackendConnection.PartitionQueryResult;
 import com.google.cloud.spanner.pgadapter.statements.IntermediateStatement;
+import com.google.cloud.spanner.pgadapter.utils.Converter;
 import com.google.cloud.spanner.pgadapter.wireoutput.CommandCompleteResponse;
 import com.google.cloud.spanner.pgadapter.wireoutput.EmptyQueryResponse;
 import com.google.cloud.spanner.pgadapter.wireoutput.ErrorResponse;
@@ -388,6 +389,7 @@ public abstract class ControlMessage extends WireMessage {
   static final class SendResultSetRunnable implements Callable<Long> {
     private final IntermediateStatement describedResult;
     private ResultSet resultSet;
+    private Converter converter;
     private final BatchReadOnlyTransaction batchReadOnlyTransaction;
     private final Partition partition;
     private final long maxRows;
@@ -428,6 +430,12 @@ public abstract class ControlMessage extends WireMessage {
         boolean hasData) {
       this.describedResult = describedResult;
       this.resultSet = resultSet;
+      this.converter =
+          new Converter(
+              describedResult,
+              mode,
+              describedResult.getConnectionHandler().getServer().getOptions(),
+              resultSet);
       this.batchReadOnlyTransaction = null;
       this.partition = null;
       this.maxRows = maxRows;
@@ -462,6 +470,12 @@ public abstract class ControlMessage extends WireMessage {
         // will be cleaned up at a later moment.
         try {
           resultSet = batchReadOnlyTransaction.execute(partition);
+          converter =
+              new Converter(
+                  describedResult,
+                  mode,
+                  describedResult.getConnectionHandler().getServer().getOptions(),
+                  resultSet);
           hasData = resultSet.next();
         } catch (Throwable t) {
           if (includePrefix) {
@@ -490,7 +504,7 @@ public abstract class ControlMessage extends WireMessage {
         if (Thread.interrupted()) {
           throw PGExceptionFactory.newQueryCancelledException();
         }
-        WireOutput wireOutput = describedResult.createDataRowResponse(resultSet, mode);
+        WireOutput wireOutput = describedResult.createDataRowResponse(converter);
         synchronized (describedResult) {
           wireOutput.send(false);
         }


Test patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java
index effdeee5..604e4ab4 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java
@@ -30,11 +30,11 @@ import com.google.cloud.spanner.connection.PostgreSQLStatementParser;
 import com.google.cloud.spanner.connection.StatementResult;
 import com.google.cloud.spanner.connection.StatementResult.ResultType;
 import com.google.cloud.spanner.pgadapter.ConnectionHandler;
-import com.google.cloud.spanner.pgadapter.ConnectionHandler.QueryMode;
 import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
 import com.google.cloud.spanner.pgadapter.metadata.DescribeMetadata;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
 import com.google.cloud.spanner.pgadapter.statements.BackendConnection.NoResult;
+import com.google.cloud.spanner.pgadapter.utils.Converter;
 import com.google.cloud.spanner.pgadapter.wireoutput.DataRowResponse;
 import com.google.cloud.spanner.pgadapter.wireoutput.WireOutput;
 import java.io.DataOutputStream;
@@ -333,8 +333,8 @@ public class IntermediateStatement {
     return EMPTY_WIRE_OUTPUT_ARRAY;
   }
 
-  public WireOutput createDataRowResponse(ResultSet resultSet, QueryMode mode) {
-    return new DataRowResponse(this.outputStream, this, resultSet, this.options, mode);
+  public WireOutput createDataRowResponse(Converter converter) {
+    return new DataRowResponse(this.outputStream, converter);
   }
 
   public WireOutput[] createResultSuffix() {