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



##########
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:
       Hi Adrien, I was thinking about the same, but I was not thinking of 
anybody using such a constant. There are several ways to check for powers of 2, 
but this one is the most elegant. Others always have multiple branches or 
checking for 0 and 1 as special cases. 
   I cann add a simple `|| alignmentBytes < 0` here, and add this as extra test 
case.
   
   This PR was already committed, so I can do this as a new commit.

##########
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:
       Offset > 0 could be checked, but as this method is mainly used inside 
IndexOutput, there is not much risk (FilePointer is always positive). But for 
separate use a check could be added.

##########
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:
       Sure, will do. The `- 1L` was already placed before the alignemntBytes, 
so it wpuld only be `addExact(offset - 1L, alignmentBytes)`, or did you have 
something else in mind?




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