You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2018/11/16 13:43:37 UTC

[arrow] branch master updated: ARROW-3194: [JAVA] Use split length in splitAndTransfer to set value count

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 074e2c2  ARROW-3194: [JAVA] Use split length in splitAndTransfer to set value count
074e2c2 is described below

commit 074e2c2971b6d0b8bde2cacca0b3fe1007497bd2
Author: siddharth <si...@dremio.com>
AuthorDate: Fri Nov 16 08:43:29 2018 -0500

    ARROW-3194: [JAVA] Use split length in splitAndTransfer to set value count
    
    (1) Use split length to set the value count of target vector in splitAndTransfer
    (2) Do not allocate memory for inner vectors in nullable map when creating transfer pairs
    (3) Add getInitReservation() API to Accountant
    
    Author: siddharth <si...@dremio.com>
    
    Closes #2958 from siddharthteotia/dremio-changes and squashes the following commits:
    
    48499ad62 <siddharth> added unit tests
    e61050158 <siddharth> ARROW-3194:  Use split length in splitAndTransfer to set valueCount
---
 .../java/org/apache/arrow/memory/Accountant.java   |  9 +++++++
 .../org/apache/arrow/memory/BufferAllocator.java   |  7 +++++
 .../org/apache/arrow/memory/TestBaseAllocator.java | 11 ++++++++
 .../arrow/vector/BaseVariableWidthVector.java      |  4 +--
 .../apache/arrow/vector/complex/StructVector.java  |  2 +-
 .../apache/arrow/vector/TestSplitAndTransfer.java  | 31 ++++++++++++++++++++--
 .../org/apache/arrow/vector/TestStructVector.java  | 18 ++++++++++++-
 7 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/java/memory/src/main/java/org/apache/arrow/memory/Accountant.java b/java/memory/src/main/java/org/apache/arrow/memory/Accountant.java
index f3d9387..28aeaf2 100644
--- a/java/memory/src/main/java/org/apache/arrow/memory/Accountant.java
+++ b/java/memory/src/main/java/org/apache/arrow/memory/Accountant.java
@@ -216,6 +216,15 @@ class Accountant implements AutoCloseable {
   }
 
   /**
+   * Return the initial reservation.
+   *
+   * @return reservation in bytes.
+   */
+  public long getInitReservation() {
+    return reservation;
+  }
+
+  /**
    * Set the maximum amount of memory that can be allocated in the this Accountant before failing
    * an allocation.
    *
diff --git a/java/memory/src/main/java/org/apache/arrow/memory/BufferAllocator.java b/java/memory/src/main/java/org/apache/arrow/memory/BufferAllocator.java
index ea2cb3a..f79b739 100644
--- a/java/memory/src/main/java/org/apache/arrow/memory/BufferAllocator.java
+++ b/java/memory/src/main/java/org/apache/arrow/memory/BufferAllocator.java
@@ -106,6 +106,13 @@ public interface BufferAllocator extends AutoCloseable {
   public long getLimit();
 
   /**
+   * Return the initial reservation.
+   *
+   * @return reservation in bytes.
+   */
+  public long getInitReservation();
+
+  /**
    * Set the maximum amount of memory this allocator is allowed to allocate.
    *
    * @param newLimit The new Limit to apply to allocations
diff --git a/java/memory/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java
index 56e85e4..c53eb06 100644
--- a/java/memory/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java
+++ b/java/memory/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java
@@ -739,6 +739,17 @@ public class TestBaseAllocator {
   }
 
   @Test
+  public void testInitReservationAndLimit() throws Exception {
+    try (final RootAllocator rootAllocator = new RootAllocator(MAX_ALLOCATION)) {
+      try (final BufferAllocator childAllocator = rootAllocator.newChildAllocator(
+              "child", 2048, 4096)) {
+        assertEquals(2048, childAllocator.getInitReservation());
+        assertEquals(4096, childAllocator.getLimit());
+      }
+    }
+  }
+
+  @Test
   public void multiple() throws Exception {
     final String owner = "test";
     try (RootAllocator allocator = new RootAllocator(Long.MAX_VALUE)) {
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
index 71f4f5b..340ccf3 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
@@ -707,8 +707,8 @@ public abstract class BaseVariableWidthVector extends BaseValueVector
     splitAndTransferValidityBuffer(startIndex, length, target);
     splitAndTransferOffsetBuffer(startIndex, length, target);
     target.setLastSet(length - 1);
-    if (this.valueCount > 0) {
-      target.setValueCount(this.valueCount);
+    if (length > 0) {
+      target.setValueCount(length);
     }
   }
 
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java
index f593a70..239a146 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java
@@ -134,7 +134,7 @@ public class StructVector extends NonNullableStructVector implements FieldVector
 
   @Override
   public TransferPair makeTransferPair(ValueVector to) {
-    return new NullableStructTransferPair(this, (StructVector) to, true);
+    return new NullableStructTransferPair(this, (StructVector) to, false);
   }
 
   @Override
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java b/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java
index fa0e03c..9689d21 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java
@@ -61,13 +61,12 @@ public class TestSplitAndTransfer {
   
       final TransferPair tp = varCharVector.getTransferPair(allocator);
       final VarCharVector newVarCharVector = (VarCharVector) tp.getTo();
-      final int[][] startLengths = {{0, 201}, {201, 200}, {401, 99}};
+      final int[][] startLengths = {{0, 201}, {201, 0}, {201, 200}, {401, 99}};
   
       for (final int[] startLength : startLengths) {
         final int start = startLength[0];
         final int length = startLength[1];
         tp.splitAndTransfer(start, length);
-        newVarCharVector.setValueCount(length);
         for (int i = 0; i < length; i++) {
           final boolean expectedSet = ((start + i) % 3) == 0;
           if (expectedSet) {
@@ -82,4 +81,32 @@ public class TestSplitAndTransfer {
       }
     }
   }
+
+  @Test
+  public void testMemoryConstrainedTransfer() {
+    try (final VarCharVector varCharVector = new VarCharVector("myvector", allocator)) {
+      allocator.setLimit(32768); /* set limit of 32KB */
+
+      varCharVector.allocateNew(10000, 1000);
+
+      final int valueCount = 1000;
+
+      for (int i = 0; i < valueCount; i += 3) {
+        final String s = String.format("%010d", i);
+        varCharVector.set(i, s.getBytes());
+      }
+      varCharVector.setValueCount(valueCount);
+
+      final TransferPair tp = varCharVector.getTransferPair(allocator);
+      final VarCharVector newVarCharVector = (VarCharVector) tp.getTo();
+      final int[][] startLengths = {{0, 700}, {700, 299}};
+
+      for (final int[] startLength : startLengths) {
+        final int start = startLength[0];
+        final int length = startLength[1];
+        tp.splitAndTransfer(start, length);
+        newVarCharVector.clear();
+      }
+    }
+  }
 }
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestStructVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestStructVector.java
index a6d5e06..54a11a3 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestStructVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestStructVector.java
@@ -17,11 +17,14 @@
 
 package org.apache.arrow.vector;
 
+import static org.junit.Assert.assertEquals;
+
 import java.util.HashMap;
 import java.util.Map;
 
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.pojo.ArrowType.Struct;
 import org.apache.arrow.vector.types.pojo.FieldType;
 import org.junit.After;
@@ -29,7 +32,6 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
-
 public class TestStructVector {
 
   private BufferAllocator allocator;
@@ -53,4 +55,18 @@ public class TestStructVector {
       Assert.assertEquals(vector.getField().getMetadata(), type.getMetadata());
     }
   }
+
+  @Test
+  public void testMakeTransferPair() {
+    try (final StructVector s1 = StructVector.empty("s1", allocator);
+         final StructVector s2 = StructVector.empty("s2", allocator)) {
+      s1.addOrGet("struct_child", FieldType.nullable(MinorType.INT.getType()), IntVector.class);
+      s1.makeTransferPair(s2);
+      final FieldVector child = s1.getChild("struct_child");
+      final FieldVector toChild = s2.addOrGet("struct_child", child.getField().getFieldType(), child.getClass());
+      assertEquals(0, toChild.getValueCapacity());
+      assertEquals(0, toChild.getDataBuffer().capacity());
+      assertEquals(0, toChild.getValidityBuffer().capacity());
+    }
+  }
 }