Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20850#discussion_r178118686
  
    --- Diff: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/codegen/UnsafeArrayWriter.java
 ---
    @@ -32,141 +30,133 @@
      */
     public final class UnsafeArrayWriter extends UnsafeWriter {
     
    -  private BufferHolder holder;
    -
    -  // The offset of the global buffer where we start to write this array.
    -  private int startingOffset;
    -
       // The number of elements in this array
       private int numElements;
     
    +  // The element size in this array
    +  private int elementSize;
    +
       private int headerInBytes;
     
       private void assertIndexIsValid(int index) {
         assert index >= 0 : "index (" + index + ") should >= 0";
         assert index < numElements : "index (" + index + ") should < " + 
numElements;
       }
     
    -  public void initialize(BufferHolder holder, int numElements, int 
elementSize) {
    +  public UnsafeArrayWriter(UnsafeWriter writer, int elementSize) {
    +    super(writer.getBufferHolder());
    +    this.elementSize = elementSize;
    +  }
    +
    +  public void initialize(int numElements) {
         // We need 8 bytes to store numElements in header
         this.numElements = numElements;
         this.headerInBytes = calculateHeaderPortionInBytes(numElements);
     
    -    this.holder = holder;
    -    this.startingOffset = holder.cursor;
    +    this.startingOffset = cursor();
     
         // Grows the global buffer ahead for header and fixed size data.
         int fixedPartInBytes =
           ByteArrayMethods.roundNumberOfBytesToNearestWord(elementSize * 
numElements);
         holder.grow(headerInBytes + fixedPartInBytes);
     
         // Write numElements and clear out null bits to header
    -    Platform.putLong(holder.buffer, startingOffset, numElements);
    +    Platform.putLong(buffer(), startingOffset, numElements);
         for (int i = 8; i < headerInBytes; i += 8) {
    -      Platform.putLong(holder.buffer, startingOffset + i, 0L);
    +      Platform.putLong(buffer(), startingOffset + i, 0L);
         }
     
         // fill 0 into reminder part of 8-bytes alignment in unsafe array
         for (int i = elementSize * numElements; i < fixedPartInBytes; i++) {
    -      Platform.putByte(holder.buffer, startingOffset + headerInBytes + i, 
(byte) 0);
    +      Platform.putByte(buffer(), startingOffset + headerInBytes + i, 
(byte) 0);
         }
    -    holder.cursor += (headerInBytes + fixedPartInBytes);
    +    incrementCursor(headerInBytes + fixedPartInBytes);
       }
     
    -  private void zeroOutPaddingBytes(int numBytes) {
    -    if ((numBytes & 0x07) > 0) {
    -      Platform.putLong(holder.buffer, holder.cursor + ((numBytes >> 3) << 
3), 0L);
    -    }
    +  protected long getOffset(int ordinal, int elementSize) {
    +    return getElementOffset(ordinal, elementSize);
       }
     
       private long getElementOffset(int ordinal, int elementSize) {
         return startingOffset + headerInBytes + ordinal * elementSize;
       }
     
    -  public void setOffsetAndSize(int ordinal, int currentCursor, int size) {
    +  @Override
    +  public void setOffsetAndSizeFromMark(int ordinal, int mark) {
         assertIndexIsValid(ordinal);
    --- End diff --
    
    If we make `setOffsetAndSizeFromMark` `final` in `UnsafeWrite`, we can not 
put `assertIndexIsValid(ordinal);` for `UnsafeArrayWrite`.
    Is it OK?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to