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 2022/05/17 23:21:57 UTC

[GitHub] [pinot] Jackie-Jiang opened a new pull request, #8722: Improve the range evaluation for raw values without range index

Jackie-Jiang opened a new pull request, #8722:
URL: https://github.com/apache/pinot/pull/8722

   Borrow the same idea as in #8703, we can reflect the inclusiveness of the value bound through the comparison value to avoid extra per value if check. Also, if the lower bound comparison already fails, we may short-circuit the comparison for the upper bound.
   
   Here are the benchmark result, each operation applies 30K values:
   Before the change:
   ```
   Benchmark                              Mode  Cnt    Score     Error  Units
   BenchmarkRangeApplySV.doubleEvaluator  avgt    5   96.914 ±   5.245  us/op
   BenchmarkRangeApplySV.floatEvaluator   avgt    5   98.179 ±   7.607  us/op
   BenchmarkRangeApplySV.intEvaluator     avgt    5  111.091 ±   1.061  us/op
   BenchmarkRangeApplySV.longEvaluator    avgt    5   95.441 ±   2.418  us/op
   BenchmarkRangeApplySV.stringEvaluator  avgt    5  467.129 ±  89.302  us/op
   ```
   After the change:
   ```
   Benchmark                              Mode  Cnt    Score     Error  Units
   BenchmarkRangeApplySV.doubleEvaluator  avgt    5   68.947 ±   1.562  us/op
   BenchmarkRangeApplySV.floatEvaluator   avgt    5   78.496 ±   5.309  us/op
   BenchmarkRangeApplySV.intEvaluator     avgt    5   66.792 ±   5.647  us/op
   BenchmarkRangeApplySV.longEvaluator    avgt    5   67.519 ±   6.929  us/op
   BenchmarkRangeApplySV.stringEvaluator  avgt    5  423.882 ± 135.095  us/op
   ```
   Benchmark class:
   ```
   @BenchmarkMode(Mode.AverageTime)
   @OutputTimeUnit(TimeUnit.MICROSECONDS)
   @Fork(1)
   @Warmup(iterations = 3, time = 10)
   @Measurement(iterations = 5, time = 10)
   @State(Scope.Benchmark)
   public class BenchmarkRangeApplySV {
     BaseRawValueBasedPredicateEvaluator _intEvaluator;
     BaseRawValueBasedPredicateEvaluator _longEvaluator;
     BaseRawValueBasedPredicateEvaluator _floatEvaluator;
     BaseRawValueBasedPredicateEvaluator _doubleEvaluator;
     BaseRawValueBasedPredicateEvaluator _stringEvaluator;
   
     @Setup
     public void setUp()
         throws Exception {
       RangePredicate rangePredicate = new RangePredicate(null, true, "10000", false, "20000");
       _intEvaluator = RangePredicateEvaluatorFactory.newRawValueBasedEvaluator(rangePredicate, FieldSpec.DataType.INT);
       _longEvaluator = RangePredicateEvaluatorFactory.newRawValueBasedEvaluator(rangePredicate, FieldSpec.DataType.LONG);
       _floatEvaluator =
           RangePredicateEvaluatorFactory.newRawValueBasedEvaluator(rangePredicate, FieldSpec.DataType.FLOAT);
       _doubleEvaluator =
           RangePredicateEvaluatorFactory.newRawValueBasedEvaluator(rangePredicate, FieldSpec.DataType.DOUBLE);
       _stringEvaluator =
           RangePredicateEvaluatorFactory.newRawValueBasedEvaluator(rangePredicate, FieldSpec.DataType.STRING);
     }
   
     @Benchmark
     public void intEvaluator(Blackhole bh) {
       for (int i = 0; i < 30000; i++) {
         bh.consume(_intEvaluator.applySV(i));
       }
     }
   
     @Benchmark
     public void longEvaluator(Blackhole bh) {
       for (int i = 0; i < 30000; i++) {
         bh.consume(_longEvaluator.applySV((long) i));
       }
     }
   
     @Benchmark
     public void floatEvaluator(Blackhole bh) {
       for (int i = 0; i < 30000; i++) {
         bh.consume(_floatEvaluator.applySV((float) i));
       }
     }
   
     @Benchmark
     public void doubleEvaluator(Blackhole bh) {
       for (int i = 0; i < 30000; i++) {
         bh.consume(_doubleEvaluator.applySV((double) i));
       }
     }
   
     @Benchmark
     public void stringEvaluator(Blackhole bh) {
       for (int i = 0; i < 30000; i++) {
         bh.consume(_stringEvaluator.applySV(Integer.toString(i)));
       }
     }
   }
   ```


-- 
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 commented on pull request #8722: Improve the range evaluation for raw values without range index

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #8722:
URL: https://github.com/apache/pinot/pull/8722#issuecomment-1129471595

   Seems it can cause wrong result for floating point values because `1.0 + Double.MIN_VALUE == 1.0` (lose precision). For the inclusive range we might need to allow some delta for floating point values similar to equals


-- 
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 commented on pull request #8722: Improve the range evaluation for raw values without range index

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #8722:
URL: https://github.com/apache/pinot/pull/8722#issuecomment-1129525090

   Solved with `Math.nextUp()` and `Math.nextDown()`


-- 
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 #8722: Improve the range evaluation for raw values without range index

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


-- 
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 #8722: Improve the range evaluation for raw values without range index

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

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8722?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 [#8722](https://codecov.io/gh/apache/pinot/pull/8722?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2cf1387) into [master](https://codecov.io/gh/apache/pinot/commit/2c3813bb9722d0ec6053bafbbdd07d7d2bd32cc3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2c3813b) will **decrease** coverage by `55.30%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8722       +/-   ##
   =============================================
   - Coverage     69.57%   14.27%   -55.31%     
   + Complexity     4572      168     -4404     
   =============================================
     Files          1729     1683       -46     
     Lines         90179    88149     -2030     
     Branches      13413    13167      -246     
   =============================================
   - Hits          62744    12580    -50164     
   - Misses        23083    74634    +51551     
   + Partials       4352      935     -3417     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `?` | |
   | unittests2 | `14.27% <0.00%> (+<0.01%)` | :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/8722?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...operator/filter/RangeIndexBasedFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvUmFuZ2VJbmRleEJhc2VkRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-70.94%)` | :arrow_down: |
   | [...lter/predicate/RangePredicateEvaluatorFactory.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL1JhbmdlUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-90.05%)` | :arrow_down: |
   | [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...in/java/org/apache/pinot/spi/utils/StringUtil.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvU3RyaW5nVXRpbC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/8722/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1352 more](https://codecov.io/gh/apache/pinot/pull/8722/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/8722?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/8722?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 [2c3813b...2cf1387](https://codecov.io/gh/apache/pinot/pull/8722?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