lidavidm commented on code in PR #43903:
URL: https://github.com/apache/arrow/pull/43903#discussion_r1741434031
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationReservation.java:
##########
@@ -59,7 +84,8 @@ public interface AllocationReservation extends AutoCloseable {
ArrowBuf allocateBuffer();
/**
- * Get the current size of the reservation (the sum of all the add()s).
+ * Get the current size of the reservation (the sum of all the add()s). Int
version is deprecated,
+ * use getLongSize instead.
Review Comment:
I don't see getLongSize?
(Also for autocomplete we might prefer getSizeLong?)
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/rounding/SegmentRoundingPolicy.java:
##########
@@ -28,15 +28,17 @@ public class SegmentRoundingPolicy implements
RoundingPolicy {
* The segment size. It must be at least {@link
SegmentRoundingPolicy#MIN_SEGMENT_SIZE}, and be a
* power of 2.
*/
- private int segmentSize;
+ private long segmentSize;
/**
* Constructor for the segment rounding policy.
*
* @param segmentSize the segment size.
* @throws IllegalArgumentException if the segment size is smaller than
{@link
* SegmentRoundingPolicy#MIN_SEGMENT_SIZE}, or is not a power of 2.
+ * @deprecated use {@link SegmentRoundingPolicy#SegmentRoundingPolicy(long)}
instead.
*/
+ @Deprecated
public SegmentRoundingPolicy(int segmentSize) {
Review Comment:
delegate this constructor
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java:
##########
@@ -929,7 +957,7 @@ public ArrowBuf allocateBuffer() {
@Override
public int getSize() {
- return nBytes;
+ return (int) nBytes;
Review Comment:
Do we have a safe cast utility? I'd rather use that than a blind cast
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java:
##########
@@ -978,6 +1006,7 @@ public void close() {
closed = true;
}
+ @Deprecated
@Override
public boolean reserve(int nBytes) {
Review Comment:
Ditto, have this method call the long version
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/rounding/DefaultRoundingPolicy.java:
##########
@@ -71,17 +71,17 @@ private static int validateAndCalculatePageShifts(int
pageSize) {
}
// Logarithm base 2. At this point we know that pageSize is a power of two.
- return Integer.SIZE - 1 - Integer.numberOfLeadingZeros(pageSize);
+ return Long.SIZE - 1 - Long.numberOfLeadingZeros(pageSize);
}
- private static int validateAndCalculateChunkSize(int pageSize, int maxOrder)
{
+ private static long validateAndCalculateChunkSize(long pageSize, long
maxOrder) {
Review Comment:
don't think maxOrder needs changing?
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java:
##########
@@ -915,6 +916,33 @@ public boolean add(final int nBytes) {
return true;
}
+ @Override
+ public boolean add(long nBytes) {
Review Comment:
I think we should cast int to long and call the other overload instead of
duplicating the code
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
