You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/01/17 10:31:46 UTC
[GitHub] [lucene] iverase opened a new pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
iverase opened a new pull request #607:
URL: https://github.com/apache/lucene/pull/607
In LUCENE-9820 we changed the API for PointValues to expose methods to navigate the BKD tree. One important change is the ability to compute the number of points below any node of the tree. In order to compute this value efficiently we need to know the shape of the tree.
Since Lucene 8.6, all trees are built as unbalanced trees but in previous version there were specialised cases where trees where built as balanced trees. This was always the case for trees with dimensions >1 and some edge cases for 1D. In order to know the tree shape for these specialised cases we need to look into the tree itself.
This change looks into how many points are in the last node of the tree for 1D lucene pre-8.6 indices. If the value is equal to the reminder of the number of points divided by the number of points per leaf the the tree is unbalanced, otherwise is balanced.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r786901423
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,46 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE) {
+ // Since lucene 8.6 all trees are unbalanced.
+ return false;
+ }
+ if (config.numDims > 1) {
+ // High dimensional tree in pre-8.6 indices are balanced.
+ return true;
+ }
+ // count of the last node for unbalanced trees
+ final int lastLeafNodePointCount = Math.toIntExact(pointCount % config.maxPointsInLeafNode);
Review comment:
I think we need to check that we have exact 1 << treeDepth nodes to be balanced
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r787379481
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,51 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE || numLeaves == 1) {
+ // since lucene 8.6 all trees are unbalanced.
+ // for only one leaf we can assume unbalanced.
+ return false;
+ }
Review comment:
This is what we are trying to solve here.
For indices created in Lucene 8.6+, they will always be unbalanced as we removed the code to build balanced trees. For one leaf, the layout for balanced and unbalanced is the same so we treat them as unbalanced. What am I missing?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r787510031
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,52 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ // for only one leaf, balanced and unbalanced trees are the same
Review comment:
nit: maybe not say "the same" but something more nuanced like "can be handled the same way" or something along these lines
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r786980842
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,46 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE) {
+ // Since lucene 8.6 all trees are unbalanced.
+ return false;
+ }
+ if (config.numDims > 1) {
+ // High dimensional tree in pre-8.6 indices are balanced.
+ return true;
+ }
+ // count of the last node for unbalanced trees
+ final int lastLeafNodePointCount = Math.toIntExact(pointCount % config.maxPointsInLeafNode);
Review comment:
done in 3490046
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r786901423
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,46 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE) {
+ // Since lucene 8.6 all trees are unbalanced.
+ return false;
+ }
+ if (config.numDims > 1) {
+ // High dimensional tree in pre-8.6 indices are balanced.
+ return true;
+ }
+ // count of the last node for unbalanced trees
+ final int lastLeafNodePointCount = Math.toIntExact(pointCount % config.maxPointsInLeafNode);
Review comment:
I think we need to check that we have exact 1 << n nodes to be balanced
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r787419417
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,51 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE || numLeaves == 1) {
+ // since lucene 8.6 all trees are unbalanced.
+ // for only one leaf we can assume unbalanced.
+ return false;
+ }
Review comment:
For Lucene pre-8.6 I added the following logic:
- if numDims > 1 then it will always be balanced.
- for numDims == 1:
if numLeaves != 1 << n, then we cannot fill the last level and therefore is unbalanced.
finally the slow path: we count the number of points I the last leaf node. If the number of points in theist leaf node == totalPoints % numLeaves then is unbalanced, otherwise balanced.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a change in pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #607:
URL: https://github.com/apache/lucene/pull/607#discussion_r786988227
##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -105,6 +107,51 @@ public BKDReader(IndexInput metaIn, IndexInput indexIn, IndexInput dataIn) throw
}
this.packedIndex = indexIn.slice("packedIndex", indexStartPointer, numIndexBytes);
this.in = dataIn;
+ this.isTreeBalanced = isTreeBalanced();
+ }
+
+ private boolean isTreeBalanced() throws IOException {
+ if (version >= BKDWriter.VERSION_META_FILE || numLeaves == 1) {
+ // since lucene 8.6 all trees are unbalanced.
+ // for only one leaf we can assume unbalanced.
+ return false;
+ }
Review comment:
I don't think this is correct. Merging has these lines of code:
```
for (PointsReader reader : mergeState.pointsReaders) {
if (reader instanceof Lucene60PointsReader == false) {
// We can only bulk merge when all to-be-merged segments use our format:
super.merge(mergeState);
return;
}
}
```
and the default merge implementation (`super.merge`) would create a balanced tree with pre-8.6, even in the 1D case.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase merged pull request #607: LUCENE-10288: Check BKD tree shape for lucene pre-8.6 1D indexes
Posted by GitBox <gi...@apache.org>.
iverase merged pull request #607:
URL: https://github.com/apache/lucene/pull/607
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org