drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject [2/3] drill git commit: DRILL-2719: ValueVector#getBuffers(clear) must consistently clear vectors & retain buffers
Date Thu, 16 Apr 2015 21:15:28 GMT
DRILL-2719: ValueVector#getBuffers(clear) must consistently clear vectors & retain buffers


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/caf779d0
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/caf779d0
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/caf779d0

Branch: refs/heads/master
Commit: caf779d014f1ddb6ac6b36693760507805b3183a
Parents: a7e9590
Author: Hanifi Gunes <hgunes@maprtech.com>
Authored: Wed Apr 8 16:11:34 2015 -0700
Committer: Mehant Baid <mehantr@gmail.com>
Committed: Thu Apr 16 10:53:34 2015 -0700

----------------------------------------------------------------------
 .../codegen/templates/NullableValueVectors.java    |  5 ++++-
 .../codegen/templates/RepeatedValueVectors.java    |  5 ++++-
 .../codegen/templates/VariableLengthVectors.java   |  6 +++++-
 .../drill/exec/vector/BaseDataValueVector.java     | 17 +++++------------
 .../exec/vector/complex/AbstractMapVector.java     | 10 ++++++++--
 .../exec/vector/complex/RepeatedListVector.java    | 10 +++++++++-
 .../exec/vector/complex/TestEmptyPopulator.java    |  2 ++
 7 files changed, 37 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java
index b4b837f..0c2341c 100644
--- a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java
+++ b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java
@@ -79,8 +79,11 @@ public final class ${className} extends BaseDataValueVector implements
<#if type
 
   @Override
   public DrillBuf[] getBuffers(boolean clear) {
-    DrillBuf[] buffers = ObjectArrays.concat(bits.getBuffers(clear), values.getBuffers(clear),
DrillBuf.class);
+    DrillBuf[] buffers = ObjectArrays.concat(bits.getBuffers(false), values.getBuffers(false),
DrillBuf.class);
     if (clear) {
+      for (DrillBuf buffer:buffers) {
+        buffer.retain();
+      }
       clear();
     }
     return buffers;

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
index c7cf8e6..e20163d 100644
--- a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
+++ b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
@@ -285,8 +285,11 @@ public final class Repeated${minor.class}Vector extends BaseValueVector
implemen
 
   @Override
   public DrillBuf[] getBuffers(boolean clear) {
-    DrillBuf[] buffers = ObjectArrays.concat(offsets.getBuffers(clear), values.getBuffers(clear),
DrillBuf.class);
+    DrillBuf[] buffers = ObjectArrays.concat(offsets.getBuffers(false), values.getBuffers(false),
DrillBuf.class);
     if (clear) {
+      for (DrillBuf buffer:buffers) {
+        buffer.retain();
+      }
       clear();
     }
     return buffers;

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
index edb851e..f2b0829 100644
--- a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
+++ b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
@@ -148,8 +148,12 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements
V
   
   @Override
   public DrillBuf[] getBuffers(boolean clear) {
-    DrillBuf[] buffers = ObjectArrays.concat(offsetVector.getBuffers(clear), super.getBuffers(clear),
DrillBuf.class);
+    DrillBuf[] buffers = ObjectArrays.concat(offsetVector.getBuffers(false), super.getBuffers(false),
DrillBuf.class);
     if (clear) {
+      // does not make much sense but we have to retain buffers even when clear is set. refactor
this interface.
+      for (DrillBuf buffer:buffers) {
+        buffer.retain();
+      }
       clear();
     }
     return buffers;

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
index d48ea99..b45dd5f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
@@ -23,7 +23,6 @@ import java.util.Iterator;
 
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.proto.UserBitShared.SerializedField;
-import org.apache.drill.exec.record.DeadBuf;
 import org.apache.drill.exec.record.MaterializedField;
 
 import com.google.common.collect.Iterators;
@@ -36,7 +35,7 @@ public abstract class BaseDataValueVector extends BaseValueVector{
 
   public BaseDataValueVector(MaterializedField field, BufferAllocator allocator) {
     super(field, allocator);
-
+    this.data = allocator.getEmpty();
   }
 
   /**
@@ -44,14 +43,8 @@ public abstract class BaseDataValueVector extends BaseValueVector{
    */
   @Override
   public void clear() {
-    if (data == null) {
-      data = DeadBuf.DEAD_BUFFER;
-    }
-    if (data != DeadBuf.DEAD_BUFFER) {
-      data.release();
-      data = data.getAllocator().getEmpty();
-      valueCount = 0;
-    }
+    data.release();
+    data = allocator.getEmpty();
   }
 
   public void setCurrentValueCount(int count) {
@@ -66,7 +59,7 @@ public abstract class BaseDataValueVector extends BaseValueVector{
   @Override
   public DrillBuf[] getBuffers(boolean clear) {
     DrillBuf[] out;
-    if (valueCount == 0) {
+    if (getBufferSize() == 0) {
       out = new DrillBuf[0];
     } else {
       out = new DrillBuf[]{data};
@@ -82,7 +75,7 @@ public abstract class BaseDataValueVector extends BaseValueVector{
   }
 
   public int getBufferSize() {
-    if (valueCount == 0) {
+    if (getAccessor().getValueCount() == 0) {
       return 0;
     }
     return data.writerIndex();

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
index b0783af..78846dc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
@@ -216,9 +216,15 @@ public abstract class AbstractMapVector extends AbstractContainerVector
{
   public DrillBuf[] getBuffers(boolean clear) {
     List<DrillBuf> buffers = Lists.newArrayList();
 
-    for (ValueVector v : vectors.values()) {
-      for (DrillBuf buf : v.getBuffers(clear)) {
+    for (ValueVector vector : vectors.values()) {
+      for (DrillBuf buf : vector.getBuffers(false)) {
         buffers.add(buf);
+        if (clear) {
+          buf.retain();
+        }
+      }
+      if (clear) {
+        vector.clear();
       }
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
index c0f5299..c061029 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
@@ -345,7 +345,15 @@ public class RepeatedListVector extends AbstractContainerVector implements
Repea
 
   @Override
   public DrillBuf[] getBuffers(boolean clear) {
-    return ArrayUtils.addAll(offsets.getBuffers(clear), vector.getBuffers(clear));
+    DrillBuf[] buffers = ArrayUtils.addAll(offsets.getBuffers(false), vector.getBuffers(false));
+    if (clear) {
+      // does not make much sense but we have to retain buffers even when clear is set. refactor
this interface.
+      for (DrillBuf buffer:buffers) {
+        buffer.retain();
+      }
+      clear();
+    }
+    return buffers;
   }
 
   protected void setVector(ValueVector newVector) {

http://git-wip-us.apache.org/repos/asf/drill/blob/caf779d0/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java
index 8426a6a..f645987 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/TestEmptyPopulator.java
@@ -43,11 +43,13 @@ public class TestEmptyPopulator extends ExecTest {
   private EmptyValuePopulator populator;
 
   private final ByteBuffer buffer = ByteBuffer.allocateDirect(BUF_SIZE);
+  private final ByteBuffer empty = ByteBuffer.allocateDirect(0);
 
 
   @Before
   public void initialize() {
     Mockito.when(allocator.buffer(Mockito.anyInt())).thenReturn(DrillBuf.wrapByteBuffer(buffer));
+    Mockito.when(allocator.getEmpty()).thenReturn(DrillBuf.wrapByteBuffer(empty));
     offsets = new UInt4Vector(null, allocator);
     offsets.allocateNewSafe();
     accessor = offsets.getAccessor();


Mime
View raw message