You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/10/22 22:35:15 UTC

[GitHub] [pinot] richardstartin opened a new pull request #7622: accelerate `ByteArray.hashCode`

richardstartin opened a new pull request #7622:
URL: https://github.com/apache/pinot/pull/7622


   ## Description
   
   Speed up `ByteArray.hashCode` which speeds up building column statistics. Unfortunately the algorithm needs to be maintained in case it has been used for partitioning, but it can be sped up easily by breaking a data dependency in the loop.
   
   ```java
   @State(Scope.Benchmark)
   public class BenchmarkByteArray {
   
     @Param({"7", "127", "1023"})
     int _size;
   
     ByteArray _bytes;
   
     @Setup(Level.Trial)
     public void init() {
       byte[] bytes = new byte[_size];
       ThreadLocalRandom.current().nextBytes(bytes);
       _bytes = new ByteArray(bytes);
     }
   
     @Benchmark
     public int byteArrayHashCode() {
       return _bytes.hashCode();
     }
   
     @Benchmark
     public int arraysHashCode() {
       return Arrays.hashCode(_bytes.getBytes());
     }
   }
   ```
   
   ```
   Benchmark                             (_size)  Mode  Cnt    Score   Error  Units
   BenchmarkByteArray.arraysHashCode           7  avgt    5    6.823 ± 3.155  ns/op
   BenchmarkByteArray.arraysHashCode         127  avgt    5   83.459 ± 5.859  ns/op
   BenchmarkByteArray.arraysHashCode        1023  avgt    5  678.600 ± 4.842  ns/op
   BenchmarkByteArray.byteArrayHashCode        7  avgt    5    6.294 ± 0.021  ns/op
   BenchmarkByteArray.byteArrayHashCode      127  avgt    5   56.796 ± 1.975  ns/op
   BenchmarkByteArray.byteArrayHashCode     1023  avgt    5  406.462 ± 3.246  ns/op
   ```
   
   
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
   * [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR fix a zero-downtime upgrade introduced earlier?
   * [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR otherwise need attention when creating release notes? Things to consider:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   * [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
   ## Release Notes
   <!-- If you have tagged this as either backward-incompat or release-notes,
   you MUST add text here that you would like to see appear in release notes of the
   next release. -->
   
   <!-- If you have a series of commits adding or enabling a feature, then
   add this section only in final commit that marks the feature completed.
   Refer to earlier release notes to see examples of text.
   -->
   ## Documentation
   <!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
   See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
   -->
   


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] Jackie-Jiang merged pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7622:
URL: https://github.com/apache/pinot/pull/7622


   


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] richardstartin commented on a change in pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#discussion_r734886249



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/utils/ByteArray.java
##########
@@ -94,7 +94,23 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Arrays.hashCode(_bytes);
+    int hash = 1;
+    int i = 0;
+    for (; i + 7 < _bytes.length; i += 8) {
+      hash = -1807454463 * hash
+          + 1742810335 * _bytes[i]
+          + 887503681 * _bytes[i + 1]
+          + 28629151 * _bytes[i + 2]
+          + 923521 * _bytes[i + 3]
+          + 29791 * _bytes[i + 4]
+          + 961 * _bytes[i + 5]
+          + 31 * _bytes[i + 6]
+          + _bytes[i + 7];
+    }
+    for (; i < _bytes.length; i++) {
+      hash = 31 * hash + _bytes[i];
+    }
+    return hash;

Review comment:
       That's not what the test does. It tests for a large number of randomly sized arrays, but needs to guarantee that the case where the main loop is skipped over and it goes straight into the post loop (length < 8).




-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] codecov-commenter edited a comment on pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#issuecomment-950016231


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7622](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4b1136) into [master](https://codecov.io/gh/apache/pinot/commit/a114cd27d61d5947ed80c6245af428f74b093dee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a114cd2) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7622/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #7622      +/-   ##
   ============================================
   - Coverage     71.61%   71.60%   -0.01%     
   - Complexity     3938     3941       +3     
   ============================================
     Files          1562     1562              
     Lines         79370    79376       +6     
     Branches      11748    11750       +2     
   ============================================
   - Hits          56843    56841       -2     
   - Misses        18692    18697       +5     
   - Partials       3835     3838       +3     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `29.37% <0.00%> (-0.06%)` | :arrow_down: |
   | integration2 | `27.85% <0.00%> (+0.09%)` | :arrow_up: |
   | unittests1 | `68.58% <100.00%> (+0.01%)` | :arrow_up: |
   | unittests2 | `14.68% <0.00%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ain/java/org/apache/pinot/spi/utils/ByteArray.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZUFycmF5LmphdmE=) | `62.50% <100.00%> (+5.35%)` | :arrow_up: |
   | [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `47.42% <0.00%> (-4.64%)` | :arrow_down: |
   | [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `80.82% <0.00%> (-2.74%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/data/TimeFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9UaW1lRmllbGRTcGVjLmphdmE=) | `88.63% <0.00%> (-2.28%)` | :arrow_down: |
   | [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `85.10% <0.00%> (-1.42%)` | :arrow_down: |
   | [.../helix/core/realtime/SegmentCompletionManager.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1NlZ21lbnRDb21wbGV0aW9uTWFuYWdlci5qYXZh) | `72.00% <0.00%> (-1.02%)` | :arrow_down: |
   | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `82.60% <0.00%> (-0.87%)` | :arrow_down: |
   | [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `67.88% <0.00%> (-0.82%)` | :arrow_down: |
   | [.../controller/helix/core/SegmentDeletionManager.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1NlZ21lbnREZWxldGlvbk1hbmFnZXIuamF2YQ==) | `75.60% <0.00%> (-0.82%)` | :arrow_down: |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `74.60% <0.00%> (-0.80%)` | :arrow_down: |
   | ... and [9 more](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a114cd2...d4b1136](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] richardstartin commented on pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#issuecomment-950014357


   > We are replacing the JDK implementation with our own, and I feel JIT might be able to optimize on the JDK default method, and the benchmark might have different result in different environment. If some new optimizations are introduced in the future java version, we won't be able to get that. Do you think this method might be slower in certain environments?
   
   As far as I am aware there is no Java or C++ compiler capable of performing this transformation, as simple as it is. It's possible that this will be treated with a vectorized intrinsic (the same trick is scalable) in the future, but Pinot doesn't run on JDK16 without adding a lot of `--add-opens` so shall we cross that bridge when it exists?


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] mqliang commented on a change in pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#discussion_r734883165



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/utils/ByteArray.java
##########
@@ -94,7 +94,23 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Arrays.hashCode(_bytes);
+    int hash = 1;
+    int i = 0;
+    for (; i + 7 < _bytes.length; i += 8) {
+      hash = -1807454463 * hash
+          + 1742810335 * _bytes[i]
+          + 887503681 * _bytes[i + 1]
+          + 28629151 * _bytes[i + 2]
+          + 923521 * _bytes[i + 3]
+          + 29791 * _bytes[i + 4]
+          + 961 * _bytes[i + 5]
+          + 31 * _bytes[i + 6]
+          + _bytes[i + 7];
+    }
+    for (; i < _bytes.length; i++) {
+      hash = 31 * hash + _bytes[i];
+    }
+    return hash;

Review comment:
       never mind, I see your test ensuring the hashcode is the same as old impl for ByteArray  




-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] richardstartin commented on a change in pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#discussion_r734883398



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/utils/ByteArray.java
##########
@@ -94,7 +94,23 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Arrays.hashCode(_bytes);
+    int hash = 1;
+    int i = 0;
+    for (; i + 7 < _bytes.length; i += 8) {
+      hash = -1807454463 * hash
+          + 1742810335 * _bytes[i]
+          + 887503681 * _bytes[i + 1]
+          + 28629151 * _bytes[i + 2]
+          + 923521 * _bytes[i + 3]
+          + 29791 * _bytes[i + 4]
+          + 961 * _bytes[i + 5]
+          + 31 * _bytes[i + 6]
+          + _bytes[i + 7];
+    }
+    for (; i < _bytes.length; i++) {
+      hash = 31 * hash + _bytes[i];
+    }
+    return hash;

Review comment:
       Or even the PR description




-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] codecov-commenter commented on pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#issuecomment-950016231


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7622](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4b1136) into [master](https://codecov.io/gh/apache/pinot/commit/a114cd27d61d5947ed80c6245af428f74b093dee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a114cd2) will **decrease** coverage by `1.24%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7622/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #7622      +/-   ##
   ============================================
   - Coverage     71.61%   70.37%   -1.25%     
   - Complexity     3938     3941       +3     
   ============================================
     Files          1562     1562              
     Lines         79370    79376       +6     
     Branches      11748    11750       +2     
   ============================================
   - Hits          56843    55859     -984     
   - Misses        18692    19694    +1002     
   + Partials       3835     3823      -12     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `27.85% <0.00%> (+0.09%)` | :arrow_up: |
   | unittests1 | `68.58% <100.00%> (+0.01%)` | :arrow_up: |
   | unittests2 | `14.68% <0.00%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ain/java/org/apache/pinot/spi/utils/ByteArray.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZUFycmF5LmphdmE=) | `62.50% <100.00%> (+5.35%)` | :arrow_up: |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-94.74%)` | :arrow_down: |
   | [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
   | [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
   | [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
   | [...ache/pinot/common/lineage/SegmentLineageUtils.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50TGluZWFnZVV0aWxzLmphdmE=) | `22.22% <0.00%> (-77.78%)` | :arrow_down: |
   | [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-77.78%)` | :arrow_down: |
   | [...verttorawindex/ConvertToRawIndexTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrR2VuZXJhdG9yLmphdmE=) | `8.77% <0.00%> (-77.20%)` | :arrow_down: |
   | ... and [98 more](https://codecov.io/gh/apache/pinot/pull/7622/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a114cd2...d4b1136](https://codecov.io/gh/apache/pinot/pull/7622?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] mqliang commented on a change in pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#discussion_r734881229



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/utils/ByteArray.java
##########
@@ -94,7 +94,23 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Arrays.hashCode(_bytes);
+    int hash = 1;
+    int i = 0;
+    for (; i + 7 < _bytes.length; i += 8) {
+      hash = -1807454463 * hash
+          + 1742810335 * _bytes[i]
+          + 887503681 * _bytes[i + 1]
+          + 28629151 * _bytes[i + 2]
+          + 923521 * _bytes[i + 3]
+          + 29791 * _bytes[i + 4]
+          + 961 * _bytes[i + 5]
+          + 31 * _bytes[i + 6]
+          + _bytes[i + 7];
+    }
+    for (; i < _bytes.length; i++) {
+      hash = 31 * hash + _bytes[i];
+    }
+    return hash;

Review comment:
       Is this implementation give the exact same hash code as `Arrays.hashCode(_bytes)`? I am mainly concern about back-comparability: say we have an existing realtime use case, to ensure we partition in the exact way as pinot, we import pinot-spi and use the old `hashCode()` impl to partition our kafka topic, after this change, will things be going wrong?




-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [pinot] mqliang commented on a change in pull request #7622: accelerate `ByteArray.hashCode`

Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7622:
URL: https://github.com/apache/pinot/pull/7622#discussion_r734885268



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/utils/ByteArray.java
##########
@@ -94,7 +94,23 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Arrays.hashCode(_bytes);
+    int hash = 1;
+    int i = 0;
+    for (; i + 7 < _bytes.length; i += 8) {
+      hash = -1807454463 * hash
+          + 1742810335 * _bytes[i]
+          + 887503681 * _bytes[i + 1]
+          + 28629151 * _bytes[i + 2]
+          + 923521 * _bytes[i + 3]
+          + 29791 * _bytes[i + 4]
+          + 961 * _bytes[i + 5]
+          + 31 * _bytes[i + 6]
+          + _bytes[i + 7];
+    }
+    for (; i < _bytes.length; i++) {
+      hash = 31 * hash + _bytes[i];
+    }
+    return hash;

Review comment:
       I am a little confused why only test ByteArray with length < 8, could you please elaborate?




-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org