xiangfu0 commented on code in PR #19539:
URL: https://github.com/apache/pinot/pull/19539#discussion_r4190590634
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SinglePartitionColumnSegmentPruner.java:
##########
@@ -152,4 +223,106 @@ private boolean isPartitionMatch(Expression
filterExpression, SegmentPartitionIn
return true;
}
}
+
+ /// Lazily prepares only visited expressions and values. Instances belong to
one prune call, never shared by queries.
+ private final class PreparedPredicate {
+ private final Expression _expression;
+ private final PartitionFunction _partitionFunction;
+ private FilterKind _kind;
+ private List<Expression> _operands;
+ private PreparedPredicate[] _children;
+ private boolean _isPartitionPredicate;
+ private Integer _singlePartitionId;
+ private IntSet _partitionIds;
+ private int _numEvaluatedValues;
+
+ private PreparedPredicate(Expression expression, PartitionFunction
partitionFunction) {
+ _expression = expression;
+ _partitionFunction = partitionFunction;
+ }
+
+ private boolean matches(Set<Integer> partitions) {
Review Comment:
I reran a bounded full-prune JMH comparison on the updated code: JDK 25, 1
thread, 2 forks, 3 × 1-second warmups and 5 × 1-second measurements per fork.
The fixture has independently constructed, equal 64-way Modulo functions and
one partition per segment; times below are µs per complete `prune()` call (mean
± JMH-reported error). “Legacy” disables preparation, “Lazy” is this PR, and
“Full eager” is a temporary, uncommitted whole-predicate ID-set variant. The
eager fixture's selected segments were checked against legacy before timing.
| Filter | Segments | Legacy | Lazy (PR) | Full eager (temporary) |
|---|---:|---:|---:|---:|
| EQUALS | 256 | 3.12 ± 0.12 | 1.70 ± 0.07 | 1.69 ± 0.07 |
| EQUALS | 4,096 | 62.28 ± 2.42 | 34.73 ± 0.95 | 36.07 ± 4.71 |
| EQUALS | 16,384 | 299.61 ± 34.65 | 150.09 ± 2.16 | 166.56 ± 40.87 |
| IN(16) | 256 | 38.13 ± 0.57 | 2.60 ± 0.03 | 2.59 ± 0.48 |
| IN(16) | 4,096 | 589.78 ± 13.44 | 48.70 ± 1.15 | 48.13 ± 4.66 |
| IN(16) | 16,384 | 2311.28 ± 408.49 | 257.33 ± 109.77 | 223.36 ± 58.37 |
| IN(128), mixed | 256 | 67.69 ± 2.51 | 5.59 ± 0.16 | 5.63 ± 2.12 |
| IN(128), mixed | 4,096 | 1156.41 ± 94.86 | 92.29 ± 11.55 | 79.59 ± 2.69 |
| IN(128), mixed | 16,384 | 5102.21 ± 693.90 | 432.47 ± 58.20 | 392.26 ±
51.51 |
| Nested AND/OR | 256 | 48.06 ± 8.61 | 6.39 ± 0.13 | 2.77 ± 0.04 |
| Nested AND/OR | 4,096 | 728.26 ± 59.33 | 118.25 ± 0.84 | 45.67 ± 0.91 |
| Nested AND/OR | 16,384 | 2873.04 ± 728.63 | 512.92 ± 9.29 | 202.74 ± 5.74 |
| IN(128), first-hit | 256 | 5.28 ± 0.04 | 3.90 ± 0.32 | 5.10 ± 0.09 |
| IN(128), first-hit | 4,096 | 95.96 ± 5.07 | 67.43 ± 4.02 | 66.36 ± 2.15 |
| IN(128), first-hit | 16,384 | 445.49 ± 13.12 | 335.03 ± 13.89 | 314.49 ±
18.70 |
The larger-segment and nested results support your point: at 4,096 and
16,384 segments, full eager reduces the nested case from 118.25 to 45.67 and
from 512.92 to 202.74 µs/op. But on a true first-literal-hit IN(128) case at
256 segments, it regresses from 3.90 to 5.10 µs/op (+31%); the 4,096/16,384
differences overlap reported error. The 16,384-segment IN(16) and some other
wide-filter cells are noisy, so I would not use them to claim a precise
crossover.
Yes, the lazy path already caches `_singlePartitionId` once per equal
function for EQUALS and single-value IN. Fully evaluating rejected
literals/branches would also change the existing short-circuit/error behavior
(covered by the invalid-literal tests). Given that and the small-case
regression, I kept the minimal lazy implementation here. This is a synthetic
broker-pruner microbenchmark, not Kraken end-to-end QPS; no benchmark source is
committed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
