jatin-bhateja commented on code in PR #1011:
URL: https://github.com/apache/parquet-mr/pull/1011#discussion_r1048036480


##########
parquet-encoding/src/main/java/org/apache/parquet/column/values/bitpacking/BytePacker.java:
##########
@@ -105,4 +116,16 @@ public void unpack8Values(final byte[] input, final int 
inPos, final int[] outpu
   public void unpack32Values(byte[] input, int inPos, int[] output, int 
outPos) {
     unpack32Values(ByteBuffer.wrap(input), inPos, output, outPos);
   }
+
+  public void unpackValuesVector(final byte[] input, final int inPos, final 
int[] output, final int outPos) {

Review Comment:
   Please add appropriate comments over newly added definitions, like the ones 
over top over scalar routines.



##########
parquet-generator/src/main/resources/ByteBitPackingVectorLE:
##########
@@ -0,0 +1,3218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.column.values.bitpacking;
+
+import jdk.incubator.vector.*;
+
+import java.nio.ByteBuffer;
+
+/**
+ * This is an auto-generated source file and should not edit it directly.
+ */
+public abstract class ByteBitPackingVectorLE {
+  private static final BytePacker[] packers = new BytePacker[33];
+
+  static {
+    packers[0] = new Packer0();
+    packers[1] = new Packer1();
+    packers[2] = new Packer2();
+    packers[3] = new Packer3();
+    packers[4] = new Packer4();
+    packers[5] = new Packer5();
+    packers[6] = new Packer6();
+    packers[7] = new Packer7();
+    packers[8] = new Packer8();
+    packers[9] = new Packer9();
+    packers[10] = new Packer10();
+    packers[11] = new Packer11();
+    packers[12] = new Packer12();
+    packers[13] = new Packer13();
+    packers[14] = new Packer14();
+    packers[15] = new Packer15();
+    packers[16] = new Packer16();
+    packers[17] = new Packer17();
+    packers[18] = new Packer18();
+    packers[19] = new Packer19();
+    packers[20] = new Packer20();
+    packers[21] = new Packer21();
+    packers[22] = new Packer22();
+    packers[23] = new Packer23();
+    packers[24] = new Packer24();
+    packers[25] = new Packer25();
+    packers[26] = new Packer26();
+    packers[27] = new Packer27();
+    packers[28] = new Packer28();
+    packers[29] = new Packer29();
+    packers[30] = new Packer30();
+    packers[31] = new Packer31();
+    packers[32] = new Packer32();
+  }
+
+  public static final BytePackerFactory factory = new BytePackerFactory() {
+    public BytePacker newBytePacker(int bitWidth) {
+      return packers[bitWidth];
+    }
+  };
+
+  private static final class Packer0 extends BytePacker {
+    private int unpackCount = 0;
+
+    private Packer0() {
+      super(0);
+    }
+
+    public int getUnpackCount() {
+      return unpackCount;
+    }
+
+    public final void pack8Values(final int[] in, final int inPos, final 
byte[] out, final int outPos) {
+    }
+
+    public final void pack32Values(final int[] in, final int inPos, final 
byte[] out, final int outPos) {
+    }
+
+    public final void unpack8Values(final byte[] in, final int inPos, final 
int[] out, final int outPos) {
+    }
+
+    public final void unpack8Values(final ByteBuffer in, final int inPos, 
final int[] out, final int outPos) {
+    }
+
+    public final void unpack32Values(final byte[] in, final int inPos, final 
int[] out, final int outPos) {
+    }
+
+    public final void unpack32Values(final ByteBuffer in, final int inPos, 
final int[] out, final int outPos) {
+    }
+
+    public final void unpackValuesVector(final byte[] input, final int inPos, 
final int[] output, final int outPos) {
+    }
+
+    public final void unpackValuesVector(final ByteBuffer input, final int 
inPos, final int[] output, final int outPos) {

Review Comment:
   We also need to ensure that we invoke both scalar generators and vector 
generators for jdk17 profile. 



##########
parquet-benchmarks/src/main/java/org/apache/parquet/benchmarks/ByteBitPackingVectorBenchmarks.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.parquet.benchmarks;
+
+import org.apache.parquet.column.values.bitpacking.BytePacker;
+import org.apache.parquet.column.values.bitpacking.Packer;
+import org.openjdk.jmh.annotations.*;
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * This class uses the java17 vector API, add VM options 
--add-modules=jdk.incubator.vector
+ */
+
+@State(Scope.Benchmark)
+@BenchmarkMode(Mode.AverageTime)
+@Warmup(iterations = 1, batchSize = 100000)
+@Measurement(iterations = 1, batchSize = 100000)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+public class ByteBitPackingVectorBenchmarks {
+
+  /**
+   * The range of bitWidth is 1 ~ 32, change it directly if test other 
bitWidth.
+   */
+  private static final int bitWidth = 7;
+  private static final int outputValues = 1024;
+  private final byte[] input = new byte[outputValues * bitWidth / 8];
+  private final int[] output = new int[outputValues];
+  private final int[] outputVector = new int[outputValues];
+
+  @Setup(Level.Trial)
+  public void getInputBytes() {
+    for (int i = 0; i < input.length; i++) {
+      input[i] = (byte) i;
+    }
+  }
+
+  @Benchmark
+  public void testUnpack() {
+    BytePacker bytePacker = Packer.LITTLE_ENDIAN.newBytePacker(bitWidth);
+    for (int i = 0, j = 0; i < input.length; i += bitWidth, j += 8) {
+      bytePacker.unpack8Values(input, i, output, j);
+    }
+  }
+
+  @Benchmark
+  public void testUnpackVector() {
+    BytePacker bytePacker = Packer.LITTLE_ENDIAN.newBytePacker(bitWidth);
+    BytePacker bytePackerVector = 
Packer.LITTLE_ENDIAN.newBytePackerVector(bitWidth);

Review Comment:
   As suggested in my following comments, it will be useful if we are able to 
access all the routines through one packer instance. 



##########
parquet-generator/src/main/resources/ByteBitPackingVectorLE:
##########
@@ -0,0 +1,3218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.parquet.column.values.bitpacking;
+
+import jdk.incubator.vector.*;
+
+import java.nio.ByteBuffer;
+
+/**
+ * This is an auto-generated source file and should not edit it directly.
+ */
+public abstract class ByteBitPackingVectorLE {
+  private static final BytePacker[] packers = new BytePacker[33];
+
+  static {
+    packers[0] = new Packer0();
+    packers[1] = new Packer1();
+    packers[2] = new Packer2();
+    packers[3] = new Packer3();
+    packers[4] = new Packer4();
+    packers[5] = new Packer5();
+    packers[6] = new Packer6();
+    packers[7] = new Packer7();
+    packers[8] = new Packer8();
+    packers[9] = new Packer9();
+    packers[10] = new Packer10();
+    packers[11] = new Packer11();
+    packers[12] = new Packer12();
+    packers[13] = new Packer13();
+    packers[14] = new Packer14();
+    packers[15] = new Packer15();
+    packers[16] = new Packer16();
+    packers[17] = new Packer17();
+    packers[18] = new Packer18();
+    packers[19] = new Packer19();
+    packers[20] = new Packer20();
+    packers[21] = new Packer21();
+    packers[22] = new Packer22();
+    packers[23] = new Packer23();
+    packers[24] = new Packer24();
+    packers[25] = new Packer25();
+    packers[26] = new Packer26();
+    packers[27] = new Packer27();
+    packers[28] = new Packer28();
+    packers[29] = new Packer29();
+    packers[30] = new Packer30();
+    packers[31] = new Packer31();
+    packers[32] = new Packer32();
+  }
+
+  public static final BytePackerFactory factory = new BytePackerFactory() {
+    public BytePacker newBytePacker(int bitWidth) {
+      return packers[bitWidth];
+    }
+  };
+
+  private static final class Packer0 extends BytePacker {
+    private int unpackCount = 0;
+
+    private Packer0() {
+      super(0);
+    }
+
+    public int getUnpackCount() {
+      return unpackCount;
+    }
+
+    public final void pack8Values(final int[] in, final int inPos, final 
byte[] out, final int outPos) {
+    }
+
+    public final void pack32Values(final int[] in, final int inPos, final 
byte[] out, final int outPos) {
+    }
+
+    public final void unpack8Values(final byte[] in, final int inPos, final 
int[] out, final int outPos) {
+    }
+
+    public final void unpack8Values(final ByteBuffer in, final int inPos, 
final int[] out, final int outPos) {
+    }
+
+    public final void unpack32Values(final byte[] in, final int inPos, final 
int[] out, final int outPos) {
+    }
+
+    public final void unpack32Values(final ByteBuffer in, final int inPos, 
final int[] out, final int outPos) {
+    }
+
+    public final void unpackValuesVector(final byte[] input, final int inPos, 
final int[] output, final int outPos) {
+    }
+
+    public final void unpackValuesVector(final ByteBuffer input, final int 
inPos, final int[] output, final int outPos) {

Review Comment:
   All these empty definitions can be removed if we introduce a new class 
ByteVectorPacker which inherit from existing BytePacker.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@parquet.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to