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


##########
lucene/core/src/java21/org/apache/lucene/internal/vectorization/PanamaVectorizationProvider.java:
##########
@@ -16,20 +16,56 @@
  */
 package org.apache.lucene.internal.vectorization;
 
+import java.io.IOException;
+import java.lang.foreign.MemorySegment;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Locale;
 import java.util.logging.Logger;
 import jdk.incubator.vector.FloatVector;
+import jdk.incubator.vector.VectorShape;
+import jdk.incubator.vector.VectorSpecies;
 import org.apache.lucene.codecs.hnsw.FlatVectorsScorer;
+import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.store.MemorySegmentAccessInput;
 import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.SuppressForbidden;
 
 /** A vectorization provider that leverages the Panama Vector API. */
 final class PanamaVectorizationProvider extends VectorizationProvider {
 
+  /** Preferred with in bits for vectors. */
+  static final int PREFERRED_VECTOR_BITSIZE;
+
+  static final VectorSpecies<Long> PRERERRED_LONG_SPECIES;
+  static final VectorSpecies<Integer> PRERERRED_INT_SPECIES;
+
+  /** Whether integer vectors can be trusted to actually be fast. */
+  static final boolean HAS_FAST_INTEGER_VECTORS;
+
   private final VectorUtilSupport vectorUtilSupport;
 
+  static {
+    // default to platform supported bitsize
+    int vectorBitSize = VectorShape.preferredShape().vectorBitSize();

Review Comment:
   In 9.x we have to be a bit of careful here, because the ctor of this class 
checks some incompatibility with security manager first. If this static code 
runs first we may hit the Java 20 bug! 
https://bugs.openjdk.org/browse/JDK-8309727
   
   We could decide on backporting if we drop support for the buggy jdk or 
possible surround this with a try/catch.



##########
lucene/core/src/java21/org/apache/lucene/internal/vectorization/PanamaVectorizationProvider.java:
##########
@@ -79,4 +113,15 @@ public VectorUtilSupport getVectorUtilSupport() {
   public FlatVectorsScorer getLucene99FlatVectorsScorer() {
     return Lucene99MemorySegmentFlatVectorsScorer.INSTANCE;
   }
+
+  @Override
+  public PostingDecodingUtil newPostingDecodingUtil(IndexInput input) throws 
IOException {
+    if (HAS_FAST_INTEGER_VECTORS && input instanceof MemorySegmentAccessInput 
msai) {
+      MemorySegment ms = msai.segmentSliceOrNull(0, input.length());

Review Comment:
   Do we really need to have a view on the full length?
   
   This may disable the wohle optimization for index files > 16 GiB (because we 
mmap them it chunks of 16 GiB).



##########
lucene/core/src/java/org/apache/lucene/codecs/lucene912/PostingIndexInput.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.codecs.lucene912;
+
+import java.io.IOException;
+import org.apache.lucene.internal.vectorization.PostingDecodingUtil;
+import org.apache.lucene.internal.vectorization.VectorizationProvider;
+import org.apache.lucene.store.IndexInput;
+
+/**
+ * Wrapper around an {@link IndexInput} and a {@link ForUtil} that optionally 
optimizes decoding
+ * using vectorization.
+ */
+public final class PostingIndexInput {

Review Comment:
   Do we really need that wrapper class? It does not hurt, but adds complexity. 
We could just get the `newPostingDecodingUtil(in)` in `Lucene912PostingsReader`.
   
   On the other hand this abstraction looks fine. If it does not change 
performance I am fine with it.



##########
lucene/core/src/java21/org/apache/lucene/internal/vectorization/PanamaVectorizationProvider.java:
##########
@@ -79,4 +113,15 @@ public VectorUtilSupport getVectorUtilSupport() {
   public FlatVectorsScorer getLucene99FlatVectorsScorer() {
     return Lucene99MemorySegmentFlatVectorsScorer.INSTANCE;
   }
+
+  @Override
+  public PostingDecodingUtil newPostingDecodingUtil(IndexInput input) throws 
IOException {
+    if (HAS_FAST_INTEGER_VECTORS && input instanceof MemorySegmentAccessInput 
msai) {
+      MemorySegment ms = msai.segmentSliceOrNull(0, input.length());

Review Comment:
   But this would only affect people with fully forcemerged indexes, by default 
our index files (more correct: segment sizes) are <= 5 GiB.
   
   We don't need any tests for this extra case, because it just falls back to 
(slower) default decoding thats used for everything else than MMapDir, and 
thats tested enough!



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