clintropolis commented on code in PR #16931:
URL: https://github.com/apache/druid/pull/16931#discussion_r1728146299
##########
processing/src/main/java/org/apache/druid/collections/spatial/ImmutableFloatNode.java:
##########
@@ -149,10 +148,24 @@ public ImmutableBitmap getImmutableBitmap()
{
final int sizePosition = initialOffset + offsetFromInitial +
HEADER_NUM_BYTES + 2 * numDims * Float.BYTES;
int numBytes = data.getInt(sizePosition);
- data.position(sizePosition + Integer.BYTES);
- ByteBuffer tmpBuffer = data.slice();
- tmpBuffer.limit(numBytes);
- return bitmapFactory.mapImmutableBitmap(tmpBuffer.asReadOnlyBuffer());
+ final ByteBuffer readOnlyBuffer = data.asReadOnlyBuffer();
+ int newPosition = sizePosition + Integer.BYTES;
+ readOnlyBuffer.position(newPosition);
+ int newLimit = readOnlyBuffer.position() + numBytes;
+ readOnlyBuffer.limit(newLimit);
+ try {
+ return bitmapFactory.mapImmutableBitmap(readOnlyBuffer);
+ }
+ catch (Exception e) {
+ log.error(e, "Failed to read bitmap from buffer '%s',"
+ + "current pos: %d, set pos: %d, set limit: %d",
+ Base64.getEncoder().encodeToString(readOnlyBuffer.array()),
+ readOnlyBuffer.position(),
+ newPosition,
+ newLimit
+ );
Review Comment:
hmm, if buffer is mmap file then `.array()` will not work, afaik it only
works for buffers that wrap a `byte[]`. Also is this log message displaying the
buffer contents that useful? Why does this need to be in a try/catch at all?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
