PARQUET-2159: Vectorized BytePacker decoder using Java VectorAPI #1011

jiangjiguang · 2022-11-15T07:32:15Z

The PR includes 3 aspects：

Use java17 vector api to decode bit-packing , the performance gain is 4x ~ 8x according to the microbenchmark
Upgrade the project to java17 to support java vector api
Add ParquetReadRouter to compatible with different platform when computing engines(such as spark) read parquet.

Jira

https://issues.apache.org/jira/projects/PARQUET/issues/PARQUET-2159?filter=allopenissues

Tests

Add unit tests org.apache.parquet.column.values.bitpacking.TestByteBitPackingVectorLE

Commits

My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

The PR adds maven profile vector to enable generate java17 vector bit-packing decode opt. code, and run junit tests: mvn clean install -P vector
The PR needs Intel Ice Lake CPU to run junit tests
The contributors are jiangjiguang jiyu1021 guangzegu Fang-Xie , and co-contributor is jatin-bhateja

jiangjiguang · 2022-11-20T11:02:18Z

@wangyum @gszadovszky This PR always fails to build, I do not know why. Is the reason of failure "1 workflow awaiting approval" ? please help me, this is my first PR to parquet-mr community, thanks!

jiangjiguang · 2022-11-24T00:56:26Z

@wangyum Could you review the PR ? thanks

gszadovszky · 2022-11-24T08:07:12Z

@jiangjiguang, Apache introduced a process to require approvals for test executions to keep resources under control. I've approved it so the tests are running. Not sure though when I will have time to properly review it.

jiangjiguang · 2022-11-24T11:02:00Z

@gszadovszky I resubmitted the code， can you approve the workflow again ?

jiangjiguang · 2022-11-25T01:28:07Z

@gszadovszky Could you review the PR ? thanks

gszadovszky · 2022-11-25T08:33:20Z

@jiangjiguang, however I agree on upgrading to newer java versions (and to have performance benefits like this one) it is not always an easy thing to do. For example our main historical user Hadoop is still on java 1.8 which means they would not be able to upgrade parquet after releasing this one. I think it worth a discussion on the dev list and maybe even a formal vote on it.

jiangjiguang · 2022-11-25T12:09:48Z

@gszadovszky I agree on it. As far as I know some products(such as hadoop presto flink ) are working towards upgrading to java17. Spark already supports java17, trino requires a minimum java version is java17.0.3, So I think parquet should support java17 as soon as possible and be compatible with java8, because of java17 vector can bring 4x ~ 8x performance gain for parquet encode/decode. The PR uses "maven profile -P vector" and "code gen" to support java17 and is compatible with java8

gszadovszky · 2022-11-25T13:45:11Z

@jiangjiguang,

What do you mean by this change is compatible with java8? The vectorized encoding/decoding requires java17 runtime, right? If yes, we will need to have a way release artifacts for java17 and keep java8 compatibility with others. We need to have an agreement (at community level) how we want to achieve this.

By running through the change related to the compile it seems that if you have a jdk17 in the compile environment it will be compiled for a java17 runtime automatically. Since we do not have a specific environment for making a release (shame on us) it highly depends on the environment of the dev how creates the release. So, we shall either add a release configuration for 8 to the jdk7 profile (if it makes sense) or do not have the jdk17 profile activated automatically. Otherwise we might release artifacts that are not compatible < java17.

jiangjiguang · 2022-11-28T01:20:34Z

@gszadovszky That what I said supports java17 and is compatible with java8 depends on compile environment. Can you start a discussion on how to support java17 ?

gszadovszky · 2022-11-28T08:43:26Z

@jiangjiguang,

With you current implementation if parquet-mr is compiled with jdk17 than it will not be compatible with java8 while any jdk under 17 will generate a code compatible with java8 since release is set to 8. My problem is this exact behaviour. I would suggest keep generating java8 compatible artifacts with using any versions of jdks (including 17) by default and let it compile for java17 (and the vectorized stuff) when specific profile is activated manually.

About starting the discussion about java17 compatibility. I am currently not really active in the community and not even following the dev list. Maybe, @shangxinli or @ggershinsky would like to drive this topic?

shangxinli · 2022-12-03T18:10:31Z

Thank @gszadovszky a lot for helping with this PR!

+1 for what @gszadovszky said. The mainstream runtime JDK is still 1.8. Parquet is one of the underlying building blocks for many big data applications. The bare minimum, for now, is to keep java8 compatible. Otherwise forcing applications to upgrade to jdk17 because of Parquet is disruptive and impacts adoptions.

@jiangjiguang, I am very happy to see you have this PR to help the Parquet community. Would you mind starting an email discussion to dev@parquet.apache.org for this topic?

cc @ggershinsky @wgtmac

wgtmac

This work looks promising! It would be great if you can add some micro-benchmark to parquet-benchmarks.

parquet-generator/src/main/resources/ByteBitPackingVectorLE

parquet-column/src/main/java/org/apache/parquet/column/values/bitpacking/ParquetReadRouter.java

parquet-encoding/src/main/java/org/apache/parquet/column/values/bitpacking/BytePacker.java

wgtmac · 2022-12-06T08:37:30Z

...src/main/java/org/apache/parquet/encoding/bitpacking/ByteBasedBitPackingVectorGenerator.java

+
+/**
+ * This class generates vector bit packers that pack the most significant bit first.
+ * The result of the generation is checked in. To regenerate the code run this class and check in the result.


Why is this intentional?

This class is used to generate ByteBitPackingVectorLE class, which unpackValues/packValues with java17 vector api.
If use java17 vector api directly, errors will occur when compiling or developing with java8(<java17).

parquet-generator/src/main/java/org/apache/parquet/encoding/GeneratorVector.java

Co-authored-by: Fang-Xie <fang.xie@intel.com> Co-authored-by: guangzegu <guangze.gu@intel.com> Co-authored-by: jiyu1021 <yu.ji@intel.com> Co-authored-by: jatin-bhateja <jatin.bhateja@intel.com>

jiangjiguang · 2022-12-14T02:40:45Z

This work looks promising! It would be great if you can add some micro-benchmark to parquet-benchmarks.

@wgtmac I have add the micro-benchmark to parquet-benchmarks, this is the result:

parquet-encoding/src/main/java/org/apache/parquet/column/values/bitpacking/BytePacker.java

parquet-generator/src/main/resources/ByteBitPackingVectorLE

...t-benchmarks/src/main/java/org/apache/parquet/benchmarks/ByteBitPackingVectorBenchmarks.java

jiangjiguang · 2022-12-14T10:21:27Z

@wgtmac I have resubmitted the PR, can you review it again ?

jiangjiguang · 2023-01-23T14:29:59Z

Thank @gszadovszky a lot for helping with this PR!

+1 for what @gszadovszky said. The mainstream runtime JDK is still 1.8. Parquet is one of the underlying building blocks for many big data applications. The bare minimum, for now, is to keep java8 compatible. Otherwise forcing applications to upgrade to jdk17 because of Parquet is disruptive and impacts adoptions.

@jiangjiguang, I am very happy to see you have this PR to help the Parquet community. Would you mind starting an email discussion to dev@parquet.apache.org for this topic?

cc @ggershinsky @wgtmac

@gszadovszky @shangxinli @wgtmac I have started the discussion about how to upgrade java17 over a month, but nobody involved! So I have updated the PR, it does not involve how to upgrade java17.
The default compilation is java8
Just add maven build parameters -P java17-target -P vector and get the expected jars when people want to use java17 vector to speed up parquet decode

gszadovszky · 2023-01-25T19:29:55Z

Sorry for the bad experience, @jiangjiguang. Unfortunately, I don't have too much time for the parquet community.
@wgtmac, @jatin-bhateja, since you were the ones who actively reviewed this PR I would like to ask for your approvals. I won't have time to properly review this but I am happy to push it if you agree.

gszadovszky · 2023-01-25T20:03:30Z

@jiangjiguang, please check test failures.

jiangjiguang · 2023-01-28T07:05:40Z

@jiangjiguang, please check test failures.

@gszadovszky Thank you for your attention about the PR, I have fixed it and the test successes.

wgtmac

Sorry for the delay. I have left some comments and the implementation is overall looking good. Thanks @jiangjiguang for your effort!

My main concern is the extensibility to support other instruction sets. In addition, it seems to me that the java vector api is still incubating. As I am not a java expert, do we have the risk of unstable API?

parquet-column/src/main/java/org/apache/parquet/column/values/bitpacking/ParquetReadRouter.java

parquet-encoding/src/main/java/org/apache/parquet/column/values/bitpacking/BytePacker.java

parquet-encoding/src/main/java/org/apache/parquet/column/values/bitpacking/Packer.java

...or/src/main/java/org/apache/parquet/encoding/vectorbitpacking/BitPackingGeneratorVector.java

pom.xml

…bitpacking/ParquetReadRouter.java Co-authored-by: Gang Wu <ustcwg@gmail.com>

wgtmac

Looks good. A minor comment mainly on naming.

wgtmac · 2023-02-28T01:18:06Z

pom.xml

@@ -659,5 +662,13 @@
        </plugins>
      </build>
    </profile>
+
+    <profile>
+      <id>plugins</id>


plugins is a little bit generic to me. Rename it to encoding-plugin or vector-plugins? Any suggestion? @gszadovszky

In addition, please rename plugins folder to parquet-plugins to follow naming of other sub-directories.

@wgtmac @gszadovszky vector-plugins +1, and it can show the feature of this PR.
I have renamed plugins to parquet-plugins.

gszadovszky

It looks good to me. I've added a note for the README, though.

One more thing. We need to add testing of this profile automatically with github actions. (The current test results for this PR have no meaning since these tests are not executed.)

README.md

jiangjiguang · 2023-02-28T13:37:57Z

@gszadovszky @wgtmac This feature need avx512vbmi and avx512_vbmi2 instruction set, so it needs github action runners with intel ice lake. I do not know how to select runners with Intel Ice Lake ? So I have submitted the help (actions/runner#2467).

jiangjiguang · 2023-03-01T05:14:57Z

@gszadovszky @wgtmac I have added a new workflow named Vector-plugins, can you run it ?

wgtmac · 2023-03-01T05:17:48Z

@gszadovszky @wgtmac I have added a new workflow named Vector-plugins, can you run it ?

It seems that this PR is closed. Could you please reopen it and see if it can run automatically?

jiangjiguang · 2023-03-01T05:37:40Z

@gszadovszky @wgtmac I have added a new workflow named Vector-plugins, can you run it ?

It seems that this PR is closed. Could you please reopen it and see if it can run automatically?

@wgtmac sorry, may be my wrong click, I have reopened it

wgtmac · 2023-03-01T05:39:17Z

@wgtmac sorry, may be my wrong click, I have reopened it

NP. Seems it is running. https://github.com/apache/parquet-mr/actions/runs/4300429406/jobs/7496638049

.github/workflows/vector-plugins.yml

wgtmac

LGTM. Thanks @jiangjiguang !

wgtmac · 2023-03-02T06:56:05Z

I'd request sign off from @gszadovszky @shangxinli

gszadovszky

I've added some comments about testing. @jiangjiguang, do you have any info about selecting the proper CPU environment for these tests?

.github/workflows/vector-plugins.yml

...src/test/java/org/apache/parquet/column/values/bitpacking/TestByteBitPacking512VectorLE.java

...-vector/src/test/java/org/apache/parquet/column/values/bitpacking/TestParquetReadRouter.java

gszadovszky

It looks good to me.
Thank you for working on it, @jiangjiguang!

jatin-bhateja · 2023-03-04T03:49:31Z

Hi @gszadovszky , @wgtmac Thanks for your reviews and guidance during this long review process. May we request to you please merge this PR by adding following people as co-authors ( @jiangjiguang @jiyu1021 @guangzegu @Fang-Xie @jatin-bhateja) alternatively make @jiangjiguang a committer so that he can merge on our behalf.

jiangjiguang · 2023-03-04T13:05:38Z

Hi @gszadovszky , @wgtmac Thanks for your reviews and guidance during this long review process. May we request to you please merge this PR by adding following people as co-authors ( @jiangjiguang @jiyu1021 @guangzegu @Fang-Xie @jatin-bhateja) alternatively make @jiangjiguang a committer so that he can merge on our behalf.

@jatin-bhateja I have added them as co-authors on the first commit.

wgtmac · 2023-03-04T13:53:51Z

I have merged it. Thanks @jiangjiguang @jatin-bhateja for the contribution and @gszadovszky for the review!

LuciferYang · 2023-04-04T08:30:15Z

@jiangjiguang Sorry to bother you, I am not sure if the use of the AVX 512 will still make other CPU cores to downshift frequency. If so, is there a way to manually turn off this feature now? Otherwise, the new version may be difficult to promote and use in the enterprise.

jiangjiguang force-pushed the PARQUET-2159 branch from a42553d to 54237fa Compare November 18, 2022 11:49

jiangjiguang force-pushed the PARQUET-2159 branch from 54237fa to 78259fd Compare November 23, 2022 11:11

jiangjiguang force-pushed the PARQUET-2159 branch from 8139fc8 to 72d868c Compare November 24, 2022 02:27

jiangjiguang force-pushed the PARQUET-2159 branch from 72d868c to 6a8cff6 Compare November 24, 2022 09:02

wgtmac reviewed Dec 6, 2022

View reviewed changes

java17 vector decode opt.

2d8cd72

Co-authored-by: Fang-Xie <fang.xie@intel.com> Co-authored-by: guangzegu <guangze.gu@intel.com> Co-authored-by: jiyu1021 <yu.ji@intel.com> Co-authored-by: jatin-bhateja <jatin.bhateja@intel.com>

jiangjiguang force-pushed the PARQUET-2159 branch from 6a8cff6 to 2d8cd72 Compare December 14, 2022 02:38

jatin-bhateja reviewed Dec 14, 2022

View reviewed changes

add java17-target profile, comments, junit

3377674

fix test case fail

c0732b4

jiangjiguang force-pushed the PARQUET-2159 branch from 32606a9 to c0732b4 Compare January 28, 2023 06:32

wgtmac reviewed Jan 28, 2023

View reviewed changes

Update parquet-column/src/main/java/org/apache/parquet/column/values/…

baed80b

…bitpacking/ParquetReadRouter.java Co-authored-by: Gang Wu <ustcwg@gmail.com>

add profile plugins

1aeaf38

wgtmac reviewed Feb 28, 2023

View reviewed changes

rename profile vector-plugins

e7c318a

gszadovszky requested changes Feb 28, 2023

View reviewed changes

README.md Show resolved Hide resolved

this feature is experimental

45efb3c

add vector-plugins workflows

4fc4c7f

jiangjiguang closed this Mar 1, 2023

jiangjiguang reopened this Mar 1, 2023

wgtmac reviewed Mar 1, 2023

View reviewed changes

.github/workflows/vector-plugins.yml Outdated Show resolved Hide resolved

.github/workflows/vector-plugins.yml Outdated Show resolved Hide resolved

vector-plugins workflows update

8653923

jiangjiguang changed the title ~~PARQUET-2159: java17 vector parquet bit-packing decode optimization~~ PARQUET-2159: Vectorized BytePacker decoder using Java VectorAPI Mar 2, 2023

vector-plugins specify modules

2df278b

wgtmac approved these changes Mar 2, 2023

View reviewed changes

gszadovszky requested changes Mar 2, 2023

View reviewed changes

Assume the vector junit

ef5d356

gszadovszky approved these changes Mar 2, 2023

View reviewed changes

fix bug

af01ac0

wgtmac merged commit d730fa7 into apache:master Mar 4, 2023

jiangjiguang mentioned this pull request Apr 3, 2023

[WIP][SPARK-42696]Speed up parquet reading with Java Vector API apache/spark#40646

Closed

jiangjiguang mentioned this pull request Apr 10, 2023

[WIP]Speed up parquet reading with Java Vector API apache/spark#40719

Closed

PARQUET-2159: Vectorized BytePacker decoder using Java VectorAPI #1011

PARQUET-2159: Vectorized BytePacker decoder using Java VectorAPI #1011

Conversation

jiangjiguang commented Nov 15, 2022

Jira

Tests

Commits

Documentation

jiangjiguang commented Nov 20, 2022

jiangjiguang commented Nov 24, 2022

gszadovszky commented Nov 24, 2022

jiangjiguang commented Nov 24, 2022

jiangjiguang commented Nov 25, 2022

gszadovszky commented Nov 25, 2022

jiangjiguang commented Nov 25, 2022

gszadovszky commented Nov 25, 2022

jiangjiguang commented Nov 28, 2022

gszadovszky commented Nov 28, 2022

shangxinli commented Dec 3, 2022 • edited

wgtmac left a comment

Choose a reason for hiding this comment

wgtmac Dec 6, 2022

Choose a reason for hiding this comment

jiangjiguang Dec 13, 2022

Choose a reason for hiding this comment

jiangjiguang commented Dec 14, 2022

jiangjiguang commented Dec 14, 2022

jiangjiguang commented Jan 23, 2023 • edited

gszadovszky commented Jan 25, 2023

gszadovszky commented Jan 25, 2023

jiangjiguang commented Jan 28, 2023 • edited

wgtmac left a comment

Choose a reason for hiding this comment

wgtmac left a comment

Choose a reason for hiding this comment

wgtmac Feb 28, 2023

Choose a reason for hiding this comment

jiangjiguang Feb 28, 2023

Choose a reason for hiding this comment

gszadovszky left a comment

Choose a reason for hiding this comment

jiangjiguang commented Feb 28, 2023

jiangjiguang commented Mar 1, 2023

wgtmac commented Mar 1, 2023

jiangjiguang commented Mar 1, 2023

wgtmac commented Mar 1, 2023

wgtmac left a comment

Choose a reason for hiding this comment

wgtmac commented Mar 2, 2023

gszadovszky left a comment

Choose a reason for hiding this comment

gszadovszky left a comment

Choose a reason for hiding this comment

jatin-bhateja commented Mar 4, 2023 • edited

jiangjiguang commented Mar 4, 2023

wgtmac commented Mar 4, 2023

LuciferYang commented Apr 4, 2023 • edited

shangxinli commented Dec 3, 2022 •

edited

jiangjiguang commented Jan 23, 2023 •

edited

jiangjiguang commented Jan 28, 2023 •

edited

jatin-bhateja commented Mar 4, 2023 •

edited

LuciferYang commented Apr 4, 2023 •

edited