gortiz commented on code in PR #15571:
URL: https://github.com/apache/pinot/pull/15571#discussion_r2063125983
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -229,4 +254,64 @@ public DataBlock visit(ErrorMseBlock block,
List<DataBuffer> serializedStats) {
}
}
}
+
+ @VisibleForTesting
+ public static List<ByteString> toByteStrings(DataBlock dataBlock, int
maxByteStringSize)
+ throws IOException {
+ return toByteStrings(dataBlock.serialize(), maxByteStringSize);
+ }
+
+ @VisibleForTesting
+ public static List<ByteString> toByteStrings(List<ByteBuffer> bytes, int
maxByteStringSize) {
+ if (bytes.isEmpty()) {
+ return EMPTY_BYTEBUFFER_LIST;
+ }
+
+ int totalBytes = 0;
+ for (ByteBuffer bb : bytes) {
+ totalBytes += bb.remaining();
+ }
+ int initialCapacity = (totalBytes / maxByteStringSize) + bytes.size();
+ List<ByteString> result = new ArrayList<>(initialCapacity);
+
+ ByteString acc = ByteString.EMPTY;
+ int available = maxByteStringSize;
+
+ for (ByteBuffer bb: bytes) {
+ int from = bb.position();
+ int remaining = bb.limit() - from;
+ while (remaining > 0) {
+ if (remaining <= available) {
+ acc = acc.concat(UnsafeByteOperations.unsafeWrap(sliceByteBuffer(bb,
from, from + remaining)));
+ available -= remaining;
+ remaining = 0;
+ } else {
+ acc = acc.concat(UnsafeByteOperations.unsafeWrap(sliceByteBuffer(bb,
from, from + available)));
+ from += available;
+ remaining -= available;
+ result.add(acc);
+ acc = ByteString.EMPTY;
+ available = maxByteStringSize;
+ }
+ }
+ }
+ result.add(acc);
+
+ return result;
+ }
+
+ // polyfill because ByteBuffer.slice(pos, lim) is not available until Java 13
+ private static ByteBuffer sliceByteBuffer(ByteBuffer bb, int position, int
limit) {
+ int oldPosition = bb.position();
+ int oldLimit = bb.limit();
+
+ try {
+ bb.position(position);
+ bb.limit(limit);
+ return bb.slice();
+ } finally {
+ bb.position(oldPosition);
+ bb.limit(oldLimit);
+ }
Review Comment:
I don't think we need this method and in fact moving position and limit all
the time may be expensive. I think we can simplify the code significantly if we
do that. For example, we won't need to use the from variable in
toByteStrings(List<ByteBuffer> bytes, int maxByteStringSize)
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -229,4 +254,64 @@ public DataBlock visit(ErrorMseBlock block,
List<DataBuffer> serializedStats) {
}
}
}
+
+ @VisibleForTesting
+ public static List<ByteString> toByteStrings(DataBlock dataBlock, int
maxByteStringSize)
Review Comment:
Do this and the overload method need to be public? Isn't protected or even
default good enough to test it?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
