uschindler commented on code in PR #15116:
URL: https://github.com/apache/lucene/pull/15116#discussion_r2295499256


##########
lucene/core/src/java/org/apache/lucene/util/GroupVIntUtil.java:
##########
@@ -57,13 +59,44 @@ public static void readGroupVInts(DataInput in, int[] dst, 
int limit) throws IOE
    * @param offset the offset in the array to start storing ints.
    */
   public static void readGroupVInt(DataInput in, int[] dst, int offset) throws 
IOException {
+    readGroupVInt(true, in, dst, offset);
+  }
+
+  /** DO not use! Only visible for benchmarking purposes! */
+  public static void readGroupVInt$Baseline(DataInput in, int[] dst, int 
offset)
+      throws IOException {
+    readGroupVInt(false, in, dst, offset);
+  }
+
+  private static void readGroupVInt(boolean optimized, DataInput in, int[] 
dst, int offset)
+      throws IOException {
     final int flag = in.readByte() & 0xFF;
 
     final int n1Minus1 = flag >> 6;
     final int n2Minus1 = (flag >> 4) & 0x03;
     final int n3Minus1 = (flag >> 2) & 0x03;
     final int n4Minus1 = flag & 0x03;
 
+    // if our DataInput implements RandomAccessInput for absolute access and 
IndexInput for seeking,
+    // we use a branch-less implementation:
+    if (optimized && in instanceof RandomAccessInput rin && in instanceof 
IndexInput iin) {

Review Comment:
   But for the future my idea would be: Enforce all `IndexInput` to allow 
positional access and remove the RandomAccessInterface.
   
   This requires a bit work for the downstream implementor, but it would avoid 
stuff like the currently unoptimized NIOFSDir. All good implementations already 
implement random access, and all the other ones create clones when random 
access is requested. This random acess is implemented by some stupid seek/read 
implementation. The clone is needed because of the seeking to not touch the 
state.
   
   If all IndexInputs needs to support random access all have to implement it 
correctly (also BufferedIndexInput can do this easily).
   
   If IndexInput is guranteed to support random access, then we can have 2 
GroupVInt implementations: One for plain DataImports and an overload for 
IndexInput that delegate to non-random one if theres not enough space till EOF.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to