[GitHub] [druid] AmatyaAvadhanula commented on a diff in pull request #12477: DimensionRangeShardSpec speed boost.

[GitBox <gi...@apache.org>]

AmatyaAvadhanula commented on code in PR #12477:
URL: https://github.com/apache/druid/pull/12477#discussion_r856884272


##########
core/src/main/java/org/apache/druid/timeline/partition/DimensionRangeShardSpec.java:
##########
@@ -223,16 +208,29 @@ public boolean possibleInDomain(Map<String, RangeSet<String>> domain)
 
       // EffectiveDomain[i] = QueryDomain[i] INTERSECTION SegmentRange[i]
       RangeSet<String> effectiveDomainForDimension = queryDomainForDimension.subRangeSet(rangeTillSegmentBoundary);
+
+      // Create an iterator to use for checking if the RangeSet is empty or is a singleton. This is significantly
+      // faster than using isEmpty() and equals(), because those methods call size() internally, which iterates
+      // the entire RangeSet.
+      final Iterator<Range<String>> effectiveDomainRangeIterator = effectiveDomainForDimension.asRanges().iterator();
+
       // Prune segment because query domain is out of segment range
-      if (effectiveDomainForDimension.isEmpty()) {
+      if (!effectiveDomainRangeIterator.hasNext()) {
         return false;
       }
 
-      // EffectiveDomain is singleton and lies only on the boundaries -> consider next dimensions
-      effectiveDomainIsStart = effectiveDomainIsStart
-                                && isRangeSetSingletonWithVal(effectiveDomainForDimension, segmentStart.get(i));
-      effectiveDomainIsEnd = effectiveDomainIsEnd
-                           && isRangeSetSingletonWithVal(effectiveDomainForDimension, segmentEnd.get(i));
+      final Range<String> firstRange = effectiveDomainRangeIterator.next();
+
+      if (!effectiveDomainRangeIterator.hasNext()) {
+        // Effective domain contained only one Range.
+        // If it's a singleton and lies only on the boundaries -> consider next dimensions
+        effectiveDomainIsStart = effectiveDomainIsStart
+                                 && segmentStart.get(i) != null
+                                 && firstRange.equals(Range.singleton(segmentStart.get(i)));
+        effectiveDomainIsEnd = effectiveDomainIsEnd
+                               && segmentEnd.get(i) != null
+                               && firstRange.equals(Range.singleton(segmentEnd.get(i)));
+      }

Review Comment:
   `else {
     return true;
   }`
   This snippet might be required at this point since effective domains are no longer singleton, and we cannot prune further?
   If not, we may go to the next dimension and prune a segment that must not be pruned.



-- 
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@druid.apache.org

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


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