You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "sherman (via GitHub)" <gi...@apache.org> on 2025/01/29 09:30:54 UTC
[I] Performance Improvement Proposal: Update Binary.hashCode Implementation [parquet-java]
sherman opened a new issue, #3139:
URL: https://github.com/apache/parquet-java/issues/3139
### Describe the enhancement requested
Hi everyone,
I've been working with Parquet and Iceberg (Java version) and encountered an interesting performance scenario involving 300 text columns, some of which are dictionary-encoded while others are not.
While profiling a write workload via Spark, I noticed that a significant amount of CPU time is spent in the **hashCode** method. Upon further investigation, I found that the **hashCode** implementation in the **Binary** class was written over 12 years ago. Given the advancements in modern JDK optimizations, I explored alternative approaches and conducted some quick benchmarks comparing the current **Binary.hashCode** method with the standard JDK implementation.
The results indicate a substantial performance improvement with the JDK method, likely due to auto-vectorization. Given these findings, I’d like to ask: Are there any specific reasons we shouldn’t consider switching to the JDK implementation?
```
Benchmark (size) Mode Cnt Score Error Units
ByteArrayHashcodeBenchmark.baseLine 4 thrpt 10 370812.768 ± 9424.228 ops/ms
ByteArrayHashcodeBenchmark.baseLine 10 thrpt 10 193567.198 ± 4076.861 ops/ms
ByteArrayHashcodeBenchmark.baseLine 16 thrpt 10 130168.452 ± 1919.892 ops/ms
ByteArrayHashcodeBenchmark.baseLine 42 thrpt 10 89942.653 ± 1258.718 ops/ms
ByteArrayHashcodeBenchmark.baseLine 128 thrpt 10 66207.606 ± 701.346 ops/ms
ByteArrayHashcodeBenchmark.binary 4 thrpt 10 265692.082 ± 2739.580 ops/ms
ByteArrayHashcodeBenchmark.binary 10 thrpt 10 131370.284 ± 506.299 ops/ms
ByteArrayHashcodeBenchmark.binary 16 thrpt 10 96812.920 ± 1285.400 ops/ms
ByteArrayHashcodeBenchmark.binary 42 thrpt 10 37953.004 ± 217.867 ops/ms
ByteArrayHashcodeBenchmark.binary 128 thrpt 10 11435.508 ± 43.145 ops/ms
```
I’d love to hear your thoughts on this!
### Component(s)
_No response_
--
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: issues-unsubscribe@parquet.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance Improvement Proposal: Update Binary.hashCode Implementation [parquet-java]
Posted by "orchestr7 (via GitHub)" <gi...@apache.org>.
orchestr7 commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2621128779
Tagging @gszadovszky @wgtmac @Fokko to bring some attention to this question
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2635830105
Yes, we need to modify some code to initialize the vectorized hashcode implementation if available. The current plugin is not distributed. So we need to manually build it and put it in the classpath like https://github.com/apache/parquet-java?tab=readme-ov-file#java-vector-api-support
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2621731830
@sherman, could you explain what do you exactly mean by "JDK implementation"? `Binary` stores offset and length for either the embedded `byte[]` or `ByteBuffer`. Since we may slice a Binary object, we need to calculate the hashCode only for the related parts of the embedded bytes. I am not aware of such a functionality existing in Java.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "sherman (via GitHub)" <gi...@apache.org>.
sherman commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2622247352
Hi, @gszadovszky !
Yes, you're absolutely right. The situation is a bit more complex, but I see a potential workaround.
Inside the JDK, there's a class called *ArraysSupport*, which provides the *vectorizedHashCode* method that we need. This method also supports additional arguments, such as *offset* and *len*, allowing it to work with array slices.
### The Problem
The ArraysSupport class is located in a module that is not exported by default. However, we can still access it by adding an extra export directive to the compiler:
```
--add-exports=java.base/jdk.internal.util=ALL-UNNAMED
```
Alternatively, a reference to the class can be obtained via a method handle.
The final implementation might look something like this:
```java
public static int hashCode(byte[] array, int offset, int length) throws Throwable {
var len = Math.max(0, length - offset);
return switch (len) {
case 0 -> 1;
case 1 -> 31 + (int) array[0];
default -> ArraysSupport.vectorizedHashCode(array, 0, array.length, 1, ArraysSupport.T_BYTE);
};
}
```
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2638880642
BTW, is it possible to leverage [Multi-Release JAR](https://openjdk.org/jeps/238)? We will eventually drop Java 8 support anyway.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2630502423
@sherman Have you considered leveraging the Vector API (though it requires Java 17)? There is a similar effort to add a plugin for vectorized bit unpacking: https://github.com/apache/parquet-java/pull/1011
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "sherman (via GitHub)" <gi...@apache.org>.
sherman commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2634910213
Hello @wgtmac!
I'm not sure I fully understand how I can use custom code (e.g., plugins) to work with the current build (incorporated in distributions like Spark/Iceberg).
It seems that if I want to use custom code, I need to modify some code at the reader level. I'm currently exploring this possibility but unsure whether the Parquet extensibility model is suitable for this purpose.
Off-topic: Before this, I worked on a search engine and was used to a lot of low-level bit-packing code and similar optimizations. Leveraging modern hardware in this way is a proper approach.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "sherman (via GitHub)" <gi...@apache.org>.
sherman commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2623996398
@akuleshov7 Yeah, this is mainly for my workload, which has a ton of string columns ;-)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "akuleshov7 (via GitHub)" <gi...@apache.org>.
akuleshov7 commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2623972279
50% increase by the performance by moving to Java 11 and Binary.hashCode? Nice!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2623883140
@sherman, Thanks for the reference. So, this class is introduced in java 11. parquet-java is still on java 8.
BTW, the `-add-exports` argument would also be required for the runtime, right? It means that every system using `parquet-java` would require to add this argument otherwise `parquet-java` won't work. I am not sure this is acceptable.
I can only imagine such an enhancement is it to be implemented in an optional module. But `Binary` is part of the core (`parquet-column` especially), so I'm not sure, how could we incorporate this.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "sherman (via GitHub)" <gi...@apache.org>.
sherman commented on issue #3139:
URL: https://github.com/apache/parquet-java/issues/3139#issuecomment-2623930734
@gszadovszky
There are two directives: --add-exports and --add-opens.
`--add-exports` allows access to all public types and members at both compile-time and runtime.
`--add-opens` enables access to non-public members at runtime.
Yes, you're right. One of those directives must also be used at runtime.
BTW, I didn’t think this would be an issue because, as far as I can see, Parquet directly uses methods from Unsafe and some NIO-related functionality, which are only accessible under the directive.
So, you can't see how this could be applied, let's close the issue.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [I] Performance improvement proposal: Update Binary.hashCode implementation [parquet-java]
Posted by "sherman (via GitHub)" <gi...@apache.org>.
sherman closed issue #3139: Performance improvement proposal: Update Binary.hashCode implementation
URL: https://github.com/apache/parquet-java/issues/3139
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org