[GitHub] [lucene] uschindler commented on a change in pull request #327: LUCENE-10125: Speed up DirectWriter.

[GitBox <gi...@apache.org>]

uschindler commented on a change in pull request #327:
URL: https://github.com/apache/lucene/pull/327#discussion_r718852626



##########
File path: lucene/core/src/java/org/apache/lucene/util/packed/DirectWriter.java
##########
@@ -94,38 +91,54 @@ private void flush() throws IOException {
     }
     // Avoid writing bits from values that are outside of the range we need to encode
     Arrays.fill(nextValues, off, nextValues.length, 0L);
-    encode(nextValues, 0, nextBlocks, 0, iterations);
+    encode(nextValues, off, nextBlocks, bitsPerValue);
     final int blockCount =
         (int) PackedInts.Format.PACKED.byteCount(PackedInts.VERSION_CURRENT, off, bitsPerValue);
     output.writeBytes(nextBlocks, blockCount);
     off = 0;
   }
 
-  public void encode(
-      long[] values, int valuesOffset, byte[] blocks, int blocksOffset, int iterations) {
-    int nextBlock = 0;
-    int bitsUsed = 0;
-    for (int i = 0; i < byteValueCount * iterations; ++i) {
-      final long v = values[valuesOffset++];
-      assert PackedInts.unsignedBitsRequired(v) <= bitsPerValue;
-      if (bitsUsed < byteOffset) {
-        // just buffer
-        nextBlock |= v << bitsUsed;
-        bitsUsed += bitsPerValue;
-      } else {
-        // flush as many blocks as possible
-        blocks[blocksOffset++] = (byte) (nextBlock | (v << bitsUsed));
-        int bits = 8 - bitsUsed;
-        while (bits <= bitsUsedOffset) {
-          blocks[blocksOffset++] = (byte) (v >> bits);
-          bits += 8;
+  private static void encode(long[] nextValues, int upTo, byte[] nextBlocks, int bitsPerValue) {
+    if ((bitsPerValue & 7) == 0) {
+      // bitsPerValue is a multiple of 8: 8, 16, 24, 32, 30, 48, 56, 64
+      final int bytesPerValue = bitsPerValue / Byte.SIZE;
+      for (int i = 0, o = 0; i < upTo; ++i, o += bytesPerValue) {

Review comment:
       Wouldn't it be better to have the if statement outside of loop? So have a separate loop per bit size?
   
   In addition we could use a casting adapter using a MethodHandle on top of the VarHandles. This would allow hotspot to have one method signature taking byte array, offset and a long, while casting is done inside the MethodHandle. This would make the signature identical and you could just switch handle but code stays.




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