clintropolis commented on code in PR #16931:
URL: https://github.com/apache/druid/pull/16931#discussion_r1724363327
##########
processing/src/main/java/org/apache/druid/collections/spatial/ImmutableFloatNode.java:
##########
@@ -182,15 +182,15 @@ public ImmutableNode<float[]> next()
numDims,
initialOffset,
data.getInt(childrenOffset + (count++) * Integer.BYTES),
- data,
+ data.duplicate(),
Review Comment:
it looks like most of the usages of `ByteBuffer` by `ImmutableFloatNode` use
calls that specify the offset in the buffer except for `getImmutableBitmap()`,
which sets the position and slices it (for some reason, it seems like this
method in general could be cleaned up). I wonder if we just change that instead
of calling duplicate inside the iterator, so that way it is a bit lazier?
```
@Override
public ImmutableBitmap getImmutableBitmap()
{
final int sizePosition = initialOffset + offsetFromInitial +
HEADER_NUM_BYTES + 2 * numDims * Float.BYTES;
int numBytes = data.getInt(sizePosition);
final ByteBuffer readOnlyBuffer = data.asReadOnlyBuffer();
readOnlyBuffer.position(sizePosition + Integer.BYTES);
readOnlyBuffer.limit(readOnlyBuffer.position() + numBytes);
return bitmapFactory.mapImmutableBitmap(readOnlyBuffer);
}
```
This is consistent with how `getCoords` works, which also does something
similar of making a readonly copy.
--
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]
