mikemccand commented on code in PR #12778:
URL: https://github.com/apache/lucene/pull/12778#discussion_r1384697358
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
+ int bytesLeft = length;
+ while (bytesLeft > 0) {
+ int bufferLeft = BYTE_BLOCK_SIZE - byteUpto;
+ if (bytesLeft < bufferLeft) { // fits within current buffer
+ copyBytes(srcPool, offset, bytesLeft);
+ break;
+ } else { // fill up this buffer and move to next one
+ if (bufferLeft > 0) {
+ copyBytes(srcPool, offset, bufferLeft);
+ bytesLeft -= bufferLeft;
+ offset += bufferLeft;
+ }
+ nextBuffer();
+ }
+ }
+ }
+
+ // copy from source pool until no bytes left. length must be fit within the
current head buffer
+ private void copyBytes(ByteBlockPool srcPool, long offset, int length) {
+ while (length > 0) {
Review Comment:
Maybe add comment that we need `while` loop here in case the byte slice to
be copied spans multiple `byte[]` buffers in the `srcPool`?
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
+ int bytesLeft = length;
+ while (bytesLeft > 0) {
+ int bufferLeft = BYTE_BLOCK_SIZE - byteUpto;
+ if (bytesLeft < bufferLeft) { // fits within current buffer
+ copyBytes(srcPool, offset, bytesLeft);
+ break;
+ } else { // fill up this buffer and move to next one
+ if (bufferLeft > 0) {
+ copyBytes(srcPool, offset, bufferLeft);
+ bytesLeft -= bufferLeft;
+ offset += bufferLeft;
+ }
+ nextBuffer();
+ }
+ }
+ }
+
+ // copy from source pool until no bytes left. length must be fit within the
current head buffer
+ private void copyBytes(ByteBlockPool srcPool, long offset, int length) {
+ while (length > 0) {
Review Comment:
Can we add an `assert` confirming `length` does indeed fit within the head
buffer?
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
+ int bytesLeft = length;
+ while (bytesLeft > 0) {
+ int bufferLeft = BYTE_BLOCK_SIZE - byteUpto;
+ if (bytesLeft < bufferLeft) { // fits within current buffer
+ copyBytes(srcPool, offset, bytesLeft);
+ break;
+ } else { // fill up this buffer and move to next one
+ if (bufferLeft > 0) {
+ copyBytes(srcPool, offset, bufferLeft);
+ bytesLeft -= bufferLeft;
+ offset += bufferLeft;
+ }
+ nextBuffer();
+ }
+ }
+ }
+
+ // copy from source pool until no bytes left. length must be fit within the
current head buffer
+ private void copyBytes(ByteBlockPool srcPool, long offset, int length) {
Review Comment:
Can we rename to make it clear the length must fit within single buffer?
Also, it's not a generic `copyBytes` but also an `append` operation?
Maybe `appendBytesSingleBuffer` or something?
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
Review Comment:
Rename to `srcOffset`?
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
+ int bytesLeft = length;
+ while (bytesLeft > 0) {
+ int bufferLeft = BYTE_BLOCK_SIZE - byteUpto;
+ if (bytesLeft < bufferLeft) { // fits within current buffer
+ copyBytes(srcPool, offset, bytesLeft);
+ break;
+ } else { // fill up this buffer and move to next one
+ if (bufferLeft > 0) {
+ copyBytes(srcPool, offset, bufferLeft);
+ bytesLeft -= bufferLeft;
+ offset += bufferLeft;
+ }
+ nextBuffer();
+ }
+ }
+ }
+
+ // copy from source pool until no bytes left. length must be fit within the
current head buffer
+ private void copyBytes(ByteBlockPool srcPool, long offset, int length) {
+ while (length > 0) {
+ byte[] bytes = srcPool.buffers[Math.toIntExact(offset >>
BYTE_BLOCK_SHIFT)];
+ int pos = Math.toIntExact(offset & BYTE_BLOCK_MASK);
+ int bytesToCopy = Math.min(length, BYTE_BLOCK_SIZE - pos);
+ System.arraycopy(bytes, pos, buffer, byteUpto, bytesToCopy);
+ length -= bytesToCopy;
+ offset += bytesToCopy;
Review Comment:
Rename to `srcOffset`?
##########
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##########
@@ -234,6 +234,44 @@ public void append(final BytesRef bytes) {
append(bytes.bytes, bytes.offset, bytes.length);
}
+ /**
+ * Append the bytes from a source {@link ByteBlockPool} at a given offset
and length
+ *
+ * @param srcPool the source pool to copy from
+ * @param offset the source pool offset
+ * @param length the number of bytes to copy
+ */
+ public void append(ByteBlockPool srcPool, long offset, int length) {
+ int bytesLeft = length;
+ while (bytesLeft > 0) {
+ int bufferLeft = BYTE_BLOCK_SIZE - byteUpto;
+ if (bytesLeft < bufferLeft) { // fits within current buffer
+ copyBytes(srcPool, offset, bytesLeft);
+ break;
+ } else { // fill up this buffer and move to next one
+ if (bufferLeft > 0) {
+ copyBytes(srcPool, offset, bufferLeft);
+ bytesLeft -= bufferLeft;
+ offset += bufferLeft;
+ }
+ nextBuffer();
+ }
+ }
+ }
+
+ // copy from source pool until no bytes left. length must be fit within the
current head buffer
+ private void copyBytes(ByteBlockPool srcPool, long offset, int length) {
+ while (length > 0) {
+ byte[] bytes = srcPool.buffers[Math.toIntExact(offset >>
BYTE_BLOCK_SHIFT)];
Review Comment:
Rename to `srcBytes`? and `srcPos`?
--
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
