[GitHub] sohami closed pull request #1427: DRILL-6674: Minor fixes to avoid auto boxing cost in logging in Later…

[GitBox <gi...@apache.org>]

sohami closed pull request #1427: DRILL-6674: Minor fixes to avoid auto boxing cost in logging in Later…
URL: https://github.com/apache/drill/pull/1427
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/LateralJoinBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/LateralJoinBatch.java
index 18843b5b4b0..ff33e2f3c05 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/LateralJoinBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/LateralJoinBatch.java
@@ -276,8 +276,9 @@ public int getRecordCount() {
    */
   @Override
   public RecordBatch getIncoming() {
-    Preconditions.checkState (left != null, "Retuning null left batch. It's unexpected since right side will only be " +
-      "called iff there is any valid left batch");
+    Preconditions.checkState (left != null,
+      "Retuning null left batch. It's unexpected since right side will only be called iff " +
+        "there is any valid left batch");
     return left;
   }
 
@@ -348,7 +349,8 @@ protected void buildSchema() throws SchemaChangeException {
     if (!prefetchFirstBatchFromBothSides()) {
       return;
     }
-    Preconditions.checkState(right.getRecordCount() == 0, "Unexpected non-empty first right batch received");
+    Preconditions.checkState(right.getRecordCount() == 0,
+      "Unexpected non-empty first right batch received");
 
     // Setup output container schema based on known left and right schema
     setupNewSchema();
@@ -728,7 +730,6 @@ private void finalizeOutputContainer() {
 
     // Set the record count in the container
     container.setRecordCount(outputIndex);
-    container.buildSchema(BatchSchema.SelectionVectorMode.NONE);
 
     batchMemoryManager.updateOutgoingStats(outputIndex);
 
@@ -976,12 +977,19 @@ private void crossJoinAndOutputRecords() {
       int leftRowId = leftJoinIndex + 1;
       int numRowsCopied = 0;
 
-      Preconditions.checkState(currentRowId <= leftRecordCount || leftJoinIndex <= leftRecordCount,
-        "Either RowId in right batch is greater than total records in left batch or all rows in left batch " +
-          "is processed but there are still rows in right batch. Details[RightRowId: %s, LeftRecordCount: %s, " +
-          "LeftJoinIndex: %s, RightJoinIndex: %s]", currentRowId, leftRecordCount, leftJoinIndex, rightJoinIndex);
+      if (currentRowId > leftRecordCount || leftJoinIndex > leftRecordCount) {
+        // Not using Preconditions.checkState here since along with condition evaluation there will be cost of boxing
+        // the arguments.
+        throw new IllegalStateException(String.format("Either RowId in right batch is greater than total records in " +
+          "left batch or all rows in left batch is processed but there are still rows in right batch. " +
+          "Details[RightRowId: %s, LeftRecordCount: %s, LeftJoinIndex: %s, RightJoinIndex: %s]",
+          currentRowId, leftRecordCount, leftJoinIndex, rightJoinIndex));
+      }
 
-      logger.trace("leftRowId and currentRowId are: {}, {}", leftRowId, currentRowId);
+      if (logger.isTraceEnabled()) {
+        // Inside the if condition to eliminate parameter boxing cost
+        logger.trace("leftRowId and currentRowId are: {}, {}", leftRowId, currentRowId);
+      }
 
       // If leftRowId matches the rowId in right row then emit left and right row. Increment outputIndex, rightJoinIndex
       // and numRowsCopied. Also set leftMatchFound to true to indicate when to increase leftJoinIndex.
@@ -1089,10 +1097,14 @@ private void setupInputOutputVectors(RecordBatch batch, int startVectorIndex, in
   private void copyDataToOutputVectors(int fromRowIndex, int toRowIndex, Map<ValueVector, ValueVector> mapping,
                                        int numRowsToCopy, boolean isRightBatch) {
     for (Map.Entry<ValueVector, ValueVector> entry : mapping.entrySet()) {
-      logger.trace("Copying data from incoming batch vector to outgoing batch vector. [IncomingBatch: " +
+
+      if (logger.isTraceEnabled()) {
+        // Inside the if condition to eliminate parameter boxing cost
+        logger.trace("Copying data from incoming batch vector to outgoing batch vector. [IncomingBatch: " +
           "(RowIndex: {}, ColumnName: {}), OutputBatch: (RowIndex: {}, ColumnName: {}) and Other: (TimeEachValue: {})]",
-        fromRowIndex, entry.getKey().getField().getName(), toRowIndex, entry.getValue().getField().getName(),
-        numRowsToCopy);
+          fromRowIndex, entry.getKey().getField().getName(), toRowIndex, entry.getValue().getField().getName(),
+          numRowsToCopy);
+      }
 
       // Copy data from input vector to output vector for numRowsToCopy times.
       for (int j = 0; j < numRowsToCopy; ++j) {
@@ -1109,8 +1121,11 @@ private void copyDataToOutputVectors(int fromRowIndex, int toRowIndex, Map<Value
    * @param numRowsToCopy - number of rows to copy from source vector to destination vectors
    */
   private void emitLeft(int leftIndex, int outIndex, int numRowsToCopy) {
-    logger.trace("Copying the left batch data. Details: [leftIndex: {}, outputIndex: {}, numsCopy: {}]",
-      leftIndex, outIndex, numRowsToCopy);
+    if (logger.isTraceEnabled()) {
+      // Inside the if condition to eliminate parameter boxing cost
+      logger.trace("Copying the left batch data. Details: [leftIndex: {}, outputIndex: {}, numsCopy: {}]",
+        leftIndex, outIndex, numRowsToCopy);
+    }
     copyDataToOutputVectors(leftIndex, outIndex, leftInputOutputVector, numRowsToCopy, false);
   }
 
@@ -1122,8 +1137,11 @@ private void emitLeft(int leftIndex, int outIndex, int numRowsToCopy) {
    * @param numRowsToCopy - number of rows to copy from source vector to destination vectors
    */
   private void emitRight(int rightIndex, int outIndex, int numRowsToCopy) {
-    logger.trace("Copying the right batch data. Details: [rightIndex: {}, outputIndex: {}, numsCopy: {}]",
-      rightIndex, outIndex, numRowsToCopy);
+    if (logger.isTraceEnabled()) {
+      // Inside the if condition to eliminate parameter boxing cost
+      logger.trace("Copying the right batch data. Details: [rightIndex: {}, outputIndex: {}, numsCopy: {}]",
+        rightIndex, outIndex, numRowsToCopy);
+    }
     copyDataToOutputVectors(rightIndex, outIndex, rightInputOutputVector, numRowsToCopy, true);
   }
 
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestRecordBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestRecordBatch.java
index 6204d37cfb0..e1b8acb42de 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestRecordBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unnest/UnnestRecordBatch.java
@@ -284,10 +284,6 @@ protected IterOutcome doWork() {
     Preconditions.checkNotNull(lateral);
     unnest.setOutputCount(memoryManager.getOutputRowCount());
     final int incomingRecordCount = incoming.getRecordCount();
-    final int currentRecord = lateral.getRecordIndex();
-    // We call this in setupSchema, but we also need to call it here so we have a reference to the appropriate vector
-    // inside of the the unnest for the current batch
-    setUnnestVector();
 
     int remainingRecordCount = unnest.getUnnestField().getAccessor().getInnerValueCount() - remainderIndex;
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services