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 2020/03/10 00:35:14 UTC

[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5132: Optimize ExpressionFilterOperator

Jackie-Jiang opened a new pull request #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132
 
 
   1. Add BYTES type and multi-value support
   2. Directly consturct DocIdSet to save the overhead of filtering
   3. Remove the redundant isMatch() for all scan based iterators
   
   Also add some TODO for future filter optimization

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5132: Optimize ExpressionFilterOperator

Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132#discussion_r403365760
 
 

 ##########
 File path: pinot-core/src/main/java/org/apache/pinot/core/operator/DocIdSetOperator.java
 ##########
 @@ -36,30 +35,20 @@
 public class DocIdSetOperator extends BaseOperator<DocIdSetBlock> {
   private static final String OPERATOR_NAME = "DocIdSetOperator";
 
-  private static final ThreadLocal<int[]> THREAD_LOCAL_DOC_IDS = new ThreadLocal<int[]>() {
-    @Override
-    protected int[] initialValue() {
-      return new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
-    }
-  };
+  private static final ThreadLocal<int[]> THREAD_LOCAL_DOC_IDS =
+      ThreadLocal.withInitial(() -> new int[DocIdSetPlanNode.MAX_DOC_PER_CALL]);
 
   private final BaseFilterOperator _filterOperator;
   private final int _maxSizeOfDocIdSet;
 
   private FilterBlockDocIdSet _filterBlockDocIdSet;
   private BlockDocIdIterator _blockDocIdIterator;
   private int _currentDocId = 0;
-  private boolean _threadLocal = true;
-
-  public DocIdSetOperator(@Nonnull BaseFilterOperator filterOperator, int maxSizeOfDocIdSet) {
-    this(filterOperator, maxSizeOfDocIdSet, true);
-  }
 
-  public DocIdSetOperator(@Nonnull BaseFilterOperator filterOperator, int maxSizeOfDocIdSet, boolean threadLocal) {
+  public DocIdSetOperator(BaseFilterOperator filterOperator, int maxSizeOfDocIdSet) {
 
 Review comment:
   This is good.
   The DocIdSet creation logic is moved outside.
   See https://github.com/apache/incubator-pinot/pull/5132/files#diff-48213157488bdb6aa43c2f6da8d2a940R131

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #5132: Optimize ExpressionFilterOperator

Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132#discussion_r403307086
 
 

 ##########
 File path: pinot-core/src/main/java/org/apache/pinot/core/operator/DocIdSetOperator.java
 ##########
 @@ -36,30 +35,20 @@
 public class DocIdSetOperator extends BaseOperator<DocIdSetBlock> {
   private static final String OPERATOR_NAME = "DocIdSetOperator";
 
-  private static final ThreadLocal<int[]> THREAD_LOCAL_DOC_IDS = new ThreadLocal<int[]>() {
-    @Override
-    protected int[] initialValue() {
-      return new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
-    }
-  };
+  private static final ThreadLocal<int[]> THREAD_LOCAL_DOC_IDS =
+      ThreadLocal.withInitial(() -> new int[DocIdSetPlanNode.MAX_DOC_PER_CALL]);
 
   private final BaseFilterOperator _filterOperator;
   private final int _maxSizeOfDocIdSet;
 
   private FilterBlockDocIdSet _filterBlockDocIdSet;
   private BlockDocIdIterator _blockDocIdIterator;
   private int _currentDocId = 0;
-  private boolean _threadLocal = true;
-
-  public DocIdSetOperator(@Nonnull BaseFilterOperator filterOperator, int maxSizeOfDocIdSet) {
-    this(filterOperator, maxSizeOfDocIdSet, true);
-  }
 
-  public DocIdSetOperator(@Nonnull BaseFilterOperator filterOperator, int maxSizeOfDocIdSet, boolean threadLocal) {
+  public DocIdSetOperator(BaseFilterOperator filterOperator, int maxSizeOfDocIdSet) {
 
 Review comment:
   this was needed rt. @fx19880617 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] codecov-io commented on issue #5132: Optimize ExpressionFilterOperator

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132#issuecomment-605729746
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=h1) Report
   > Merging [#5132](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1f1baf85fb318caa48b3b207e032946ddd5de11c&el=desc) will **decrease** coverage by `0.03%`.
   > The diff coverage is `68.57%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/5132/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #5132      +/-   ##
   ============================================
   - Coverage     65.90%   65.87%   -0.04%     
     Complexity       12       12              
   ============================================
     Files          1052     1056       +4     
     Lines         54170    54027     -143     
     Branches       8078     8045      -33     
   ============================================
   - Hits          35702    35591     -111     
   + Misses        15819    15802      -17     
   + Partials       2649     2634      -15     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `71.97% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `92.68% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `78.84% <ø> (-2.31%)` | `0.00 <0.00> (ø)` | |
   | [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (ø)` | `0.00 <0.00> (?)` | |
   | ... and [182 more](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=footer). Last update [00fcb1d...aaa35fa](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] fx19880617 merged pull request #5132: Optimize ExpressionFilterOperator

Posted by GitBox <gi...@apache.org>.
fx19880617 merged pull request #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5132: Optimize ExpressionFilterOperator

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5132: Optimize ExpressionFilterOperator
URL: https://github.com/apache/incubator-pinot/pull/5132#issuecomment-605729746
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=h1) Report
   > Merging [#5132](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1f1baf85fb318caa48b3b207e032946ddd5de11c&el=desc) will **decrease** coverage by `0.03%`.
   > The diff coverage is `68.57%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/5132/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #5132      +/-   ##
   ============================================
   - Coverage     65.90%   65.87%   -0.04%     
     Complexity       12       12              
   ============================================
     Files          1052     1056       +4     
     Lines         54170    54027     -143     
     Branches       8078     8045      -33     
   ============================================
   - Hits          35702    35591     -111     
   + Misses        15819    15802      -17     
   + Partials       2649     2634      -15     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `71.97% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `92.68% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `78.84% <ø> (-2.31%)` | `0.00 <0.00> (ø)` | |
   | [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (ø)` | `0.00 <0.00> (?)` | |
   | ... and [182 more](https://codecov.io/gh/apache/incubator-pinot/pull/5132/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=footer). Last update [00fcb1d...aaa35fa](https://codecov.io/gh/apache/incubator-pinot/pull/5132?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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