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 2021/11/25 11:11:39 UTC

[GitHub] [lucene] iverase opened a new pull request #476: LUCENE-10262: Lift up restrictions for navigating PointValues#PointTree

iverase opened a new pull request #476:
URL: https://github.com/apache/lucene/pull/476


   see https://issues.apache.org/jira/browse/LUCENE-10262


-- 
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 #476: LUCENE-10262: Lift up restrictions for navigating PointValues#PointTree

Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #476:
URL: https://github.com/apache/lucene/pull/476#discussion_r757001712



##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -297,11 +301,20 @@ public boolean moveToChild() throws IOException {
       if (isLeafNode()) {
         return false;
       }
+      maybeResetNodeDataPosition();
       pushBoundsLeft();
       pushLeft();
       return true;
     }
 
+    private void maybeResetNodeDataPosition() throws IOException {
+      if (readNodeDataPositions[level] != innerNodes.getFilePointer()) {
+        // move position of the inner nodes index in case we move to the first child
+        assert readNodeDataPositions[level] < innerNodes.getFilePointer();
+        innerNodes.seek(readNodeDataPositions[level]);
+      }

Review comment:
       yes, quick check and it makes no difference.

##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -515,15 +529,20 @@ public void addAll(PointValues.IntersectVisitor visitor, boolean grown) throws I
 
     @Override
     public void visitDocValues(PointValues.IntersectVisitor visitor) throws IOException {
+      maybeResetNodeDataPosition();
+      addOneByOne(visitor);
+    }
+
+    public void addOneByOne(PointValues.IntersectVisitor visitor) throws IOException {

Review comment:
       done




-- 
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 #476: LUCENE-10262: Lift up restrictions for navigating PointValues#PointTree

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #476:
URL: https://github.com/apache/lucene/pull/476#discussion_r756919845



##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -297,11 +301,20 @@ public boolean moveToChild() throws IOException {
       if (isLeafNode()) {
         return false;
       }
+      maybeResetNodeDataPosition();
       pushBoundsLeft();
       pushLeft();
       return true;
     }
 
+    private void maybeResetNodeDataPosition() throws IOException {
+      if (readNodeDataPositions[level] != innerNodes.getFilePointer()) {
+        // move position of the inner nodes index in case we move to the first child
+        assert readNodeDataPositions[level] < innerNodes.getFilePointer();
+        innerNodes.seek(readNodeDataPositions[level]);
+      }

Review comment:
       seek should be cheap, I wonder if we could call it `resetNodeDataPosition()` and change the impl to call seek unconditionally?

##########
File path: lucene/core/src/java/org/apache/lucene/util/bkd/BKDReader.java
##########
@@ -515,15 +529,20 @@ public void addAll(PointValues.IntersectVisitor visitor, boolean grown) throws I
 
     @Override
     public void visitDocValues(PointValues.IntersectVisitor visitor) throws IOException {
+      maybeResetNodeDataPosition();
+      addOneByOne(visitor);
+    }
+
+    public void addOneByOne(PointValues.IntersectVisitor visitor) throws IOException {

Review comment:
       Maybe give it a more explicit name, e.g. `visitLeavesOneByOne`?




-- 
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 #476: LUCENE-10262: Lift up restrictions for navigating PointValues#PointTree

Posted by GitBox <gi...@apache.org>.
iverase merged pull request #476:
URL: https://github.com/apache/lucene/pull/476


   


-- 
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