apacheGH-15187: Make reader instantiation thread safe in BaseValueVector

rtadepalli · Jun 4, 2023 · 76219f5 · 76219f5
1 parent 778b26f
commit 76219f5
Show file tree

Hide file tree

Showing 42 changed files with 198 additions and 576 deletions.
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java
@@ -17,13 +17,15 @@
 
 package org.apache.arrow.vector;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.Collections;
 import java.util.Iterator;
 
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.ReferenceManager;
 import org.apache.arrow.util.Preconditions;
+import org.apache.arrow.vector.complex.reader.FieldReader;
 import org.apache.arrow.vector.util.DataSizeRoundingUtil;
 import org.apache.arrow.vector.util.TransferPair;
 import org.apache.arrow.vector.util.ValueVectorUtility;
@@ -50,6 +52,8 @@ public abstract class BaseValueVector implements ValueVector {
 
   protected final BufferAllocator allocator;
 
+  protected volatile FieldReader fieldReader;
+
   protected BaseValueVector(BufferAllocator allocator) {
     this.allocator = Preconditions.checkNotNull(allocator, "allocator cannot be null");
   }
@@ -143,6 +147,43 @@ long computeCombinedBufferSize(int valueCount, int typeWidth) {
     return allocator.getRoundingPolicy().getRoundedSize(bufferSize);
   }
 
+  /**
+   * Each vector has a different reader that implements the FieldReader interface. Overridden methods must make
+   * sure to return the correct type of the reader implementation to instantiate the reader properly.
+   *
+   * @return Returns the implementation class type of the vector's reader.
+   */
+  protected abstract Class<? extends FieldReader> getReaderImplClass();
+
+  /**
+   * Default implementation to create a reader for the vector. Depends on the individual vector
+   * class' implementation of `getReaderImpl()` to initialize the reader appropriately.
+   *
+   * @return Concrete instance of FieldReader by using lazy initialization.
+   */
+  public FieldReader getReader() {
+    FieldReader reader = fieldReader;
+
+    if (reader != null) {
+      return reader;
+    }
+    synchronized (this) {
+      if (fieldReader == null) {
+        try {
+          fieldReader =
+              (FieldReader) Class.forName(getReaderImplClass().getName()).getConstructor(getClass())
+                  .newInstance(this);
+        } catch (ClassNotFoundException | NoSuchMethodException | InstantiationException | IllegalAccessException |
+                 InvocationTargetException e) {
+          logger.error("Unable to instantiate FieldReader for {} because of: ", getClass().getSimpleName(), e);
+          throw new RuntimeException(e);
+        }
+      }
+
+      return fieldReader;
+    }
+  }
+
   /**
    * Container for primitive vectors (1 for the validity bit-mask and one to hold the values).
    */

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BigIntVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BigIntVector.java
@@ -19,8 +19,6 @@
 
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
-import java.util.function.Supplier;
-
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.complex.impl.BigIntReaderImpl;
@@ -39,7 +37,6 @@
  */
 public final class BigIntVector extends BaseFixedWidthVector implements BaseIntVector {
   public static final byte TYPE_WIDTH = 8;
-  private Supplier<FieldReader> reader;
 
   /**
    * Instantiate a BigIntVector. This doesn't allocate any memory for
@@ -73,21 +70,11 @@ public BigIntVector(String name, FieldType fieldType, BufferAllocator allocator)
    */
   public BigIntVector(Field field, BufferAllocator allocator) {
     super(field, allocator, TYPE_WIDTH);
-    reader = () -> {
-      final FieldReader fieldReader = new BigIntReaderImpl(BigIntVector.this);
-      reader = () -> fieldReader;
-      return fieldReader;
-    };
   }
 
-  /**
-   * Get a reader that supports reading values from this vector.
-   *
-   * @return Field Reader for this vector
-   */
   @Override
-  public FieldReader getReader() {
-    return reader.get();
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    return BigIntReaderImpl.class;
   }
 
   /**
@@ -292,7 +279,7 @@ public static long get(final ArrowBuf buffer, final int index) {
 
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param ref name of the target vector

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
@@ -20,8 +20,6 @@
 import static org.apache.arrow.memory.util.LargeMemoryUtil.capAtMaxInt;
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
-import java.util.function.Supplier;
-
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.util.ArrowBufPointer;
@@ -48,8 +46,6 @@ public final class BitVector extends BaseFixedWidthVector {
 
   private static final int HASH_CODE_FOR_ONE = 19;
 
-  private Supplier<FieldReader> reader;
-
   /**
    * Instantiate a BitVector. This doesn't allocate any memory for
    * the data in vector.
@@ -82,21 +78,11 @@ public BitVector(String name, FieldType fieldType, BufferAllocator allocator) {
    */
   public BitVector(Field field, BufferAllocator allocator) {
     super(field, allocator, 0);
-    reader = () -> {
-      final FieldReader fieldReader = new BitReaderImpl(BitVector.this);
-      reader = () -> fieldReader;
-      return fieldReader;
-    };
   }
 
-  /**
-   * Get a reader that supports reading values from this vector.
-   *
-   * @return Field Reader for this vector
-   */
   @Override
-  public FieldReader getReader() {
-    return reader.get();
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    return BitReaderImpl.class;
   }
 
   /**
@@ -548,7 +534,7 @@ public void setRangeToOne(int firstBitIndex, int count) {
 
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param ref       name of the target vector
@@ -561,7 +547,7 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
   }
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param field Field object used by the target vector

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/DateDayVector.java b/java/vector/src/main/java/org/apache/arrow/vector/DateDayVector.java
@@ -19,8 +19,6 @@
 
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
-import java.util.function.Supplier;
-
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.complex.impl.DateDayReaderImpl;
@@ -40,7 +38,6 @@
 public final class DateDayVector extends BaseFixedWidthVector {
 
   public static final byte TYPE_WIDTH = 4;
-  private Supplier<FieldReader> reader;
 
   /**
    * Instantiate a DateDayVector. This doesn't allocate any memory for
@@ -74,21 +71,11 @@ public DateDayVector(String name, FieldType fieldType, BufferAllocator allocator
    */
   public DateDayVector(Field field, BufferAllocator allocator) {
     super(field, allocator, TYPE_WIDTH);
-    reader = () -> {
-      final FieldReader fieldReader = new DateDayReaderImpl(DateDayVector.this);
-      reader = () -> fieldReader;
-      return fieldReader;
-    };
   }
 
-  /**
-   * Get a reader that supports reading values from this vector.
-   *
-   * @return Field Reader for this vector
-   */
   @Override
-  public FieldReader getReader() {
-    return reader.get();
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    return DateDayReaderImpl.class;
   }
 
   /**
@@ -296,7 +283,7 @@ public static int get(final ArrowBuf buffer, final int index) {
 
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param ref name of the target vector
@@ -309,7 +296,7 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
   }
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param field Field object used by the target vector

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/DateMilliVector.java b/java/vector/src/main/java/org/apache/arrow/vector/DateMilliVector.java
@@ -20,7 +20,6 @@
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
 import java.time.LocalDateTime;
-import java.util.function.Supplier;
 
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
@@ -41,7 +40,6 @@
  */
 public final class DateMilliVector extends BaseFixedWidthVector {
   public static final byte TYPE_WIDTH = 8;
-  private Supplier<FieldReader> reader;
 
   /**
    * Instantiate a DateMilliVector. This doesn't allocate any memory for
@@ -75,21 +73,11 @@ public DateMilliVector(String name, FieldType fieldType, BufferAllocator allocat
    */
   public DateMilliVector(Field field, BufferAllocator allocator) {
     super(field, allocator, TYPE_WIDTH);
-    reader = () -> {
-      final FieldReader fieldReader = new DateMilliReaderImpl(DateMilliVector.this);
-      reader = () -> fieldReader;
-      return fieldReader;
-    };
   }
 
-  /**
-   * Get a reader that supports reading values from this vector.
-   *
-   * @return Field Reader for this vector
-   */
   @Override
-  public FieldReader getReader() {
-    return reader.get();
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    return DateMilliReaderImpl.class;
   }
 
   /**
@@ -298,7 +286,7 @@ public static long get(final ArrowBuf buffer, final int index) {
 
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param ref name of the target vector
@@ -311,7 +299,7 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
   }
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param field Field object used by the target vector

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/java/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java
@@ -21,7 +21,6 @@
 
 import java.math.BigDecimal;
 import java.nio.ByteOrder;
-import java.util.function.Supplier;
 
 import org.apache.arrow.memory.ArrowBuf;
 import org.apache.arrow.memory.BufferAllocator;
@@ -47,7 +46,6 @@ public final class Decimal256Vector extends BaseFixedWidthVector {
   public static final int MAX_PRECISION = 76;
   public static final byte TYPE_WIDTH = 32;
   private static final boolean LITTLE_ENDIAN = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN;
-  private Supplier<FieldReader> reader;
 
   private final int precision;
   private final int scale;
@@ -86,23 +84,13 @@ public Decimal256Vector(String name, FieldType fieldType, BufferAllocator alloca
   public Decimal256Vector(Field field, BufferAllocator allocator) {
     super(field, allocator, TYPE_WIDTH);
     ArrowType.Decimal arrowType = (ArrowType.Decimal) field.getFieldType().getType();
-    reader = () -> {
-      final FieldReader fieldReader = new Decimal256ReaderImpl(Decimal256Vector.this);
-      reader = () -> fieldReader;
-      return fieldReader;
-    };
     this.precision = arrowType.getPrecision();
     this.scale = arrowType.getScale();
   }
 
-  /**
-   * Get a reader that supports reading values from this vector.
-   *
-   * @return Field Reader for this vector
-   */
   @Override
-  public FieldReader getReader() {
-    return reader.get();
+  protected Class<? extends FieldReader> getReaderImplClass() {
+    return Decimal256ReaderImpl.class;
   }
 
   /**
@@ -537,7 +525,7 @@ public void setSafe(int index, int isSet, long start, ArrowBuf buffer) {
 
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param ref name of the target vector
@@ -550,7 +538,7 @@ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
   }
 
   /**
-   * Construct a TransferPair comprising of this and a target vector of
+   * Construct a TransferPair comprising this and a target vector of
    * the same type.
    *
    * @param field Field object used by the target vector