[GitHub] [lucene] jpountz commented on a change in pull request #203: LUCENE-10019: Align file starts in CFS files to have proper alignment (8 bytes)

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

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



##########
File path: lucene/core/src/java/org/apache/lucene/store/IndexOutput.java
##########
@@ -73,4 +73,34 @@ public String getName() {
   public String toString() {
     return resourceDescription;
   }
+
+  /**
+   * Aligns the current file pointer to multiples of {@code alignmentBytes} bytes to improve reads
+   * with mmap. This will write between 0 and {@code (alignmentBytes-1)} zero bytes using {@link
+   * #writeByte(byte)}.
+   *
+   * @param alignmentBytes the alignment to which it should forward file pointer (must be a power of
+   *     2)
+   * @return the new file pointer after alignment
+   * @see #alignOffset(long, int)
+   */
+  public final long alignFilePointer(int alignmentBytes) throws IOException {
+    final long offset = getFilePointer(), alignedOffset = alignOffset(offset, alignmentBytes);
+    final int count = (int) (alignedOffset - offset);
+    for (int i = 0; i < count; i++) {
+      writeByte((byte) 0);
+    }
+    return alignedOffset;
+  }
+
+  /**
+   * Aligns the given {@code offset} to multiples of {@code alignmentBytes} bytes by rounding up.
+   * The alignment must be a power of 2.
+   */
+  public static final long alignOffset(long offset, int alignmentBytes) {
+    if (1 != Integer.bitCount(alignmentBytes)) {
+      throw new IllegalArgumentException("Alignment must be a power of 2");
+    }

Review comment:
       Being extra paranoid, but maybe verify that `offset >= 0` and that `alignmentBytes` is not `Integer#MIN_VALUE` too?

##########
File path: lucene/core/src/java/org/apache/lucene/store/IndexOutput.java
##########
@@ -73,4 +73,34 @@ public String getName() {
   public String toString() {
     return resourceDescription;
   }
+
+  /**
+   * Aligns the current file pointer to multiples of {@code alignmentBytes} bytes to improve reads
+   * with mmap. This will write between 0 and {@code (alignmentBytes-1)} zero bytes using {@link
+   * #writeByte(byte)}.
+   *
+   * @param alignmentBytes the alignment to which it should forward file pointer (must be a power of
+   *     2)
+   * @return the new file pointer after alignment
+   * @see #alignOffset(long, int)
+   */
+  public final long alignFilePointer(int alignmentBytes) throws IOException {
+    final long offset = getFilePointer(), alignedOffset = alignOffset(offset, alignmentBytes);
+    final int count = (int) (alignedOffset - offset);
+    for (int i = 0; i < count; i++) {
+      writeByte((byte) 0);
+    }
+    return alignedOffset;
+  }
+
+  /**
+   * Aligns the given {@code offset} to multiples of {@code alignmentBytes} bytes by rounding up.
+   * The alignment must be a power of 2.
+   */
+  public static final long alignOffset(long offset, int alignmentBytes) {
+    if (1 != Integer.bitCount(alignmentBytes)) {
+      throw new IllegalArgumentException("Alignment must be a power of 2");
+    }
+    return (offset - 1L + alignmentBytes) & (-alignmentBytes);

Review comment:
       Maybe use addExact to detect overflows?




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