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/04/07 07:44:11 UTC

[GitHub] [lucene] jpountz commented on a change in pull request #69: LUCENE-9850: Use PFOR encoding for doc IDs (instead of FOR)

jpountz commented on a change in pull request #69:
URL: https://github.com/apache/lucene/pull/69#discussion_r608411103



##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/PForUtil.java
##########
@@ -35,6 +46,7 @@ static boolean allEqual(long[] l) {
   }
 
   private final ForUtil forUtil;
+  private final byte[] exceptionBuff = new byte[14];

Review comment:
       Maybe we should extract a constant for the max number of exceptions so that we could then do something like that here, to make reading easier?
   ```suggestion
     private final byte[] exceptionBuff = new byte[MAX_EXCEPTIONS/2];
   ```

##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/PForUtil.java
##########
@@ -121,4 +167,146 @@ void skip(DataInput in) throws IOException {
       in.skipBytes(forUtil.numBytes(bitsPerValue) + (numExceptions << 1));
     }
   }
+
+  /**
+   * Fill {@code longs} with the final values for the case of all deltas being 1. Note this assumes
+   * there are no exceptions to apply.
+   */
+  private static void prefixSumOfOnes(long[] longs, long base) {
+    System.arraycopy(IDENTITY_PLUS_ONE, 0, longs, 0, ForUtil.BLOCK_SIZE);
+    // This loop gets auto-vectorized
+    for (int i = 0; i < ForUtil.BLOCK_SIZE; ++i) {
+      longs[i] += base;
+    }
+  }
+
+  /**
+   * Fill {@code longs} with the final values for the case of all deltas being {@code val}. Note
+   * this assumes there are no exceptions to apply.
+   */
+  private static void prefixSumOf(long[] longs, long base, long val) {
+    for (int i = 0; i < ForUtil.BLOCK_SIZE; i++) {
+      longs[i] = (i + 1) * val + base;

Review comment:
       have you looked into whether the JVM auto-vectorized this? If it doesn't I wonder if we could trick it into vectorizing by doing something like that instead
   
   ```suggestion
         longs[i] = IDENTITY_PLUS_ONE[i] * val + base;
   ```

##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/ForUtil.java
##########
@@ -513,6 +513,108 @@ void decode(int bitsPerValue, DataInput in, long[] longs) throws IOException {
     }
   }
 
+  /**
+   * Decodes 128 integers into 64 {@code longs} such that each long contains two values, each
+   * represented with 32 bits. Values [0..63] are encoded in the high-order bits of {@code longs}
+   * [0..63], and values [64..127] are encoded in the low-order bits of {@code longs} [0..63]. This
+   * representation may allow subsequent operations to be performed on two values at a time.
+   */
+  void decodeTo32(int bitsPerValue, DataInput in, long[] longs) throws IOException {
+    switch (bitsPerValue) {
+      case 1:
+        decode1(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 2:
+        decode2(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 3:
+        decode3(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 4:
+        decode4(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 5:
+        decode5(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 6:
+        decode6(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 7:
+        decode7(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 8:
+        decode8(in, tmp, longs);
+        expand8To32(longs);
+        break;
+      case 9:
+        decode9(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 10:
+        decode10(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 11:
+        decode11(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 12:
+        decode12(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 13:
+        decode13(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 14:
+        decode14(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 15:
+        decode15(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 16:
+        decode16(in, tmp, longs);
+        expand16To32(longs);
+        break;
+      case 17:
+        decode17(in, tmp, longs);
+        break;
+      case 18:
+        decode18(in, tmp, longs);
+        break;
+      case 19:
+        decode19(in, tmp, longs);
+        break;
+      case 20:
+        decode20(in, tmp, longs);
+        break;
+      case 21:
+        decode21(in, tmp, longs);
+        break;
+      case 22:
+        decode22(in, tmp, longs);
+        break;
+      case 23:
+        decode23(in, tmp, longs);
+        break;
+      case 24:
+        decode24(in, tmp, longs);
+        break;
+      default:
+        decodeSlow(bitsPerValue, in, tmp, longs);
+        break;
+    }
+  }
+

Review comment:
       can you update the `lucene/core/src/java/org/apache/lucene/codecs/lucene90/gen_ForUtil.py` file so that it would include this change when regenerating the file?




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

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