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



##########
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:
       @jpountz The idea with a functional interface like in #333 was also my 
idea how to simplify DirectReader, which has code duplication. All 
DirectReaderXY classes have exact same copypasted code, only the get method is 
different. I looked at this a few months ago and then forget about it. 
   
   My idea would be to only hae a single and final DirectReader class which 
gets instantiated with a private functional interface to implement get(). I 
just had no time, but this would simplify code a bit. Instead of those many 
inner classes, you would just have many static methods "get12()", "get48()" and 
use those with a method reference.




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