thomasmueller commented on code in PR #3076:
URL: https://github.com/apache/jackrabbit-oak/pull/3076#discussion_r4245541637
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndex.java:
##########
@@ -127,14 +146,14 @@ private static PropertyIndexPlan createPlan(NodeState
root, Filter filter,
if (PROPERTY.equals(definition.getString(TYPE_PROPERTY_NAME))
&& definition.hasChildNode(INDEX_CONTENT_NODE_NAME)) {
PropertyIndexPlan plan = new PropertyIndexPlan(
- entry.getName(), root, definition, filter,
mountInfoProvider);
+ entry.getName(), root, definition, filter,
mountInfoProvider, disableConfigurableCosts);
if (plan.getCost() != Double.POSITIVE_INFINITY) {
LOG.debug("property cost for {} is {}",
plan.getName(), plan.getCost());
if (bestPlan == null || plan.getCost() <
bestPlan.getCost()) {
bestPlan = plan;
- // Stop comparing if the costs are the minimum
- if (plan.getCost() == PropertyIndexPlan.COST_OVERHEAD)
{
+ // Stop comparing if the cost can't possibly be beaten
+ if (plan.getCost() == bestPossibleCost) {
Review Comment:
I think we can simplify the PR if we use the following instead:
```suggestion
// Stop comparing if the cost can't possibly be
beaten
if (plan.getCost() <=
PropertyIndexPlan.COST_OVERHEAD) {
```
And then, we don't need the bestPossibleCost field.
Reason: with the new behavior, the cost can get lower than the cost overhead
(which is about 2 I think: extremely low cost). But this we don't really care
much: the important part is that we can stop cost estimation work once we found
an index with extremely low cost.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -168,22 +163,22 @@ public class PropertyIndexPlan {
}
}
values = PropertyIndexUtil.encode(values);
- double cost = strategies.isEmpty() ? MAX_COST : 0;
+ long count = strategies.isEmpty() ? MAX_COUNT : 0;
for (IndexStoreStrategy strategy : strategies) {
- cost += strategy.count(filter, root, definition,
- values, MAX_COST);
+ count += strategy.count(filter, root, definition,
+ values, MAX_COUNT);
Review Comment:
As count is a long now, we would need saturating add, so we have a guarantee
there is no integer overflow.
--
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]
