leventov commented on a change in pull request #5957: Various changes
URL: https://github.com/apache/incubator-druid/pull/5957#discussion_r202521387
##########
File path:
processing/src/main/java/io/druid/collections/spatial/ImmutableRTree.java
##########
@@ -75,7 +74,7 @@ public static ImmutableRTree newImmutableFromMutable(RTree
rTree)
return empty();
}
- ByteBuffer buffer = ByteBuffer.wrap(new byte[calcNumBytes(rTree)]);
+ ByteBuffer buffer = ByteBuffer.allocate(calcNumBytes(rTree));
Review comment:
Direct ByteBuffers are not the thing that "magically makes everything
faster". There are two main reasons to use direct ByteBuffers:
- Reduce the size of Java Heap, (ordinary BBs are backed with `byte[]`
arrays which are in the heap), to reduce GC pause times. Yes, even heap
populated with arrays of primitives could affect GC pause times negatively.
However, this comes with many downsides:
- Native allocations are slower
- Native allocations contribute to potentially unrecoverable
fragmentation. On the contrary, Java Heap fragmentation is cleaned up, sooner
or later.
- You need ensure timely freeing of direct BBs, preferably with
try-with-resources, not relying on reference cleaning
- Avoid extra data copies when going native, particularly when a ByteBuffer
participate network or disk IO calls (ByteChannel.read() / write())
Neither of this directly applies to `ImmutableRTree`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
