This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new ccc79e9c69 GH-37286: [Java] Start adding nullability/nullness
annotations (#37723)
ccc79e9c69 is described below
commit ccc79e9c6946501806cfb2a6b6ea86d4d0097df1
Author: david dali susanibar arce <[email protected]>
AuthorDate: Tue Jan 9 09:18:55 2024 -0500
GH-37286: [Java] Start adding nullability/nullness annotations (#37723)
### Rationale for this change
Closes: https://github.com/apache/arrow/issues/37286
### What changes are included in this PR?
Initial support for:
- Use the Checker Framework to enhances Java’s type system to make it more
powerful and useful. Planning to start with [Nullness
Checker](https://checkerframework.org/manual/#nullness-checker)
### Are these changes tested?
These are the activities involved on this PR:
- [x] Configure the Checker Framework
- [x] Treat checker errors as warnings initially
- [x] Applying Nullness Checker annotation as needed: @ NonNull / @ Nullable
- [x] Check if building timer increases after this checker is added
- [x] Fixes for code review
### Are there any user-facing changes?
Yes
* Closes: #37286
Lead-authored-by: david dali susanibar arce <[email protected]>
Co-authored-by: David Susanibar Arce <[email protected]>
Co-authored-by: David Susanibar Arce <[email protected]>
Signed-off-by: David Li <[email protected]>
---
java/memory/memory-core/pom.xml | 45 +++++
.../java/org/apache/arrow/memory/Accountant.java | 14 +-
.../org/apache/arrow/memory/AllocationManager.java | 12 +-
.../org/apache/arrow/memory/AllocationOutcome.java | 7 +-
.../arrow/memory/AllocationOutcomeDetails.java | 5 +-
.../java/org/apache/arrow/memory/ArrowBuf.java | 39 ++---
.../org/apache/arrow/memory/BaseAllocator.java | 189 +++++++++++++--------
.../org/apache/arrow/memory/BufferAllocator.java | 3 +-
.../java/org/apache/arrow/memory/BufferLedger.java | 106 ++++++------
.../memory/DefaultAllocationManagerOption.java | 9 +-
.../arrow/memory/LowCostIdentityHashMap.java | 37 ++--
.../apache/arrow/memory/util/ArrowBufPointer.java | 9 +-
.../apache/arrow/memory/util/HistoricalLog.java | 13 +-
.../org/apache/arrow/memory/util/MemoryUtil.java | 15 +-
.../org/apache/arrow/memory/util/StackTrace.java | 27 +--
.../arrow/memory/util/hash/MurmurHasher.java | 3 +-
.../arrow/memory/util/hash/SimpleHasher.java | 3 +-
.../java/org/apache/arrow/util/Preconditions.java | 6 +
java/pom.xml | 7 +
19 files changed, 357 insertions(+), 192 deletions(-)
diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml
index 8f28699045..b914b1fa10 100644
--- a/java/memory/memory-core/pom.xml
+++ b/java/memory/memory-core/pom.xml
@@ -35,6 +35,10 @@
<groupId>org.immutables</groupId>
<artifactId>value</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.checkerframework</groupId>
+ <artifactId>checker-qual</artifactId>
+ </dependency>
</dependencies>
<build>
@@ -90,5 +94,46 @@
</plugins>
</build>
</profile>
+
+ <profile>
+ <id>checkerframework-jdk11+</id>
+ <activation>
+ <jdk>[11,]</jdk>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <source>8</source>
+ <target>8</target>
+ <encoding>UTF-8</encoding>
+ <compilerArgs combine.children="append">
+ <arg>-Xmaxerrs</arg> <!-- javac only reports the first 100
errors or warnings -->
+ <arg>10000</arg>
+ <arg>-Xmaxwarns</arg>
+ <arg>10000</arg>
+ <arg>-AskipDefs=.*Test</arg> <!-- Skip analysis for Testing
classes -->
+ <arg>-AatfDoNotCache</arg> <!-- not cache results -->
+ </compilerArgs>
+ <annotationProcessorPaths combine.children="append">
+ <path>
+ <groupId>org.checkerframework</groupId>
+ <artifactId>checker</artifactId>
+ <version>${checker.framework.version}</version>
+ </path>
+ </annotationProcessorPaths>
+ <annotationProcessors>
+ <!-- To support @Value.Immutable processors -->
+
<annotationProcessor>org.immutables.value.internal.$processor$.$Processor</annotationProcessor>
+ <!-- Add all the checkers you want to enable here -->
+
<annotationProcessor>org.checkerframework.checker.nullness.NullnessChecker</annotationProcessor>
+ </annotationProcessors>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
</profiles>
</project>
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java
index 87769dd122..b87f1345a5 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java
@@ -22,6 +22,7 @@ import java.util.concurrent.atomic.AtomicLong;
import javax.annotation.concurrent.ThreadSafe;
import org.apache.arrow.util.Preconditions;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Provides a concurrent way to manage account for memory usage without
locking. Used as basis
@@ -34,7 +35,7 @@ class Accountant implements AutoCloseable {
/**
* The parent allocator.
*/
- protected final Accountant parent;
+ protected final @Nullable Accountant parent;
private final String name;
@@ -59,7 +60,7 @@ class Accountant implements AutoCloseable {
*/
private final AtomicLong locallyHeldMemory = new AtomicLong();
- public Accountant(Accountant parent, String name, long reservation, long
maxAllocation) {
+ public Accountant(@Nullable Accountant parent, String name, long
reservation, long maxAllocation) {
Preconditions.checkNotNull(name, "name must not be null");
Preconditions.checkArgument(reservation >= 0, "The initial reservation
size must be non-negative.");
Preconditions.checkArgument(maxAllocation >= 0, "The maximum allocation
limit must be non-negative.");
@@ -73,12 +74,13 @@ class Accountant implements AutoCloseable {
this.allocationLimit.set(maxAllocation);
if (reservation != 0) {
+ Preconditions.checkArgument(parent != null, "parent must not be null");
// we will allocate a reservation from our parent.
final AllocationOutcome outcome = parent.allocateBytes(reservation);
if (!outcome.isOk()) {
throw new OutOfMemoryException(String.format(
- "Failure trying to allocate initial reservation for Allocator. " +
- "Attempted to allocate %d bytes.", reservation),
outcome.getDetails());
+ "Failure trying to allocate initial reservation for Allocator.
" +
+ "Attempted to allocate %d bytes.", reservation),
outcome.getDetails());
}
}
}
@@ -103,7 +105,7 @@ class Accountant implements AutoCloseable {
}
}
- private AllocationOutcome.Status allocateBytesInternal(long size,
AllocationOutcomeDetails details) {
+ private AllocationOutcome.Status allocateBytesInternal(long size, @Nullable
AllocationOutcomeDetails details) {
final AllocationOutcome.Status status = allocate(size,
true /*incomingUpdatePeek*/, false /*forceAllocation*/, details);
if (!status.isOk()) {
@@ -168,7 +170,7 @@ class Accountant implements AutoCloseable {
* @return The outcome of the allocation.
*/
private AllocationOutcome.Status allocate(final long size, final boolean
incomingUpdatePeak,
- final boolean forceAllocation, AllocationOutcomeDetails details) {
+ final boolean forceAllocation, @Nullable AllocationOutcomeDetails
details) {
final long oldLocal = locallyHeldMemory.getAndAdd(size);
final long newLocal = oldLocal + size;
// Borrowed from Math.addExact (but avoid exception here)
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java
index 3071c02f30..6ccefdd9c1 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java
@@ -18,6 +18,7 @@
package org.apache.arrow.memory;
import org.apache.arrow.util.Preconditions;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* An AllocationManager is the implementation of a physical memory allocation.
@@ -48,8 +49,9 @@ public abstract class AllocationManager {
// This is mostly a semantic constraint on the API user: if the reference
count reaches 0 in the owningLedger, then
// there are not supposed to be any references through other allocators. In
practice, this doesn't do anything
// as the implementation just forces ownership to be transferred to one of
the other extant references.
- private volatile BufferLedger owningLedger;
+ private volatile @Nullable BufferLedger owningLedger;
+ @SuppressWarnings("nullness:method.invocation") //call to associate(a, b)
not allowed on the given receiver
protected AllocationManager(BufferAllocator accountingAllocator) {
Preconditions.checkNotNull(accountingAllocator);
accountingAllocator.assertOpen();
@@ -61,7 +63,7 @@ public abstract class AllocationManager {
this.owningLedger = associate(accountingAllocator, false);
}
- BufferLedger getOwningLedger() {
+ @Nullable BufferLedger getOwningLedger() {
return owningLedger;
}
@@ -133,9 +135,9 @@ public abstract class AllocationManager {
// remove the <BaseAllocator, BufferLedger> mapping for the allocator
// of calling BufferLedger
Preconditions.checkState(map.containsKey(allocator),
- "Expecting a mapping for allocator and reference manager");
+ "Expecting a mapping for allocator and reference manager");
final BufferLedger oldLedger = map.remove(allocator);
-
+ Preconditions.checkState(oldLedger != null, "Expecting a mapping for
allocator and reference manager");
BufferAllocator oldAllocator = oldLedger.getAllocator();
if (oldAllocator instanceof BaseAllocator) {
// needed for debug only: tell the allocator that AllocationManager is
removing a
@@ -168,7 +170,7 @@ public abstract class AllocationManager {
// the release call was made by a non-owning reference manager, so after
remove there have
// to be 1 or more <allocator, reference manager> mappings
Preconditions.checkState(map.size() > 0,
- "The final removal of reference manager should be connected to
owning reference manager");
+ "The final removal of reference manager should be connected to
owning reference manager");
}
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java
index 2977775e6c..21a57eee49 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java
@@ -19,15 +19,18 @@ package org.apache.arrow.memory;
import java.util.Optional;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+
/**
* Describes the type of outcome that occurred when trying to account for
allocation of memory.
*/
public class AllocationOutcome {
private final Status status;
- private final AllocationOutcomeDetails details;
+ private final @Nullable AllocationOutcomeDetails details;
static final AllocationOutcome SUCCESS_INSTANCE = new
AllocationOutcome(Status.SUCCESS);
- AllocationOutcome(Status status, AllocationOutcomeDetails details) {
+ AllocationOutcome(Status status, @Nullable AllocationOutcomeDetails details)
{
this.status = status;
this.details = details;
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java
index 6499ce84b1..3ceda71cce 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java
@@ -20,6 +20,9 @@ package org.apache.arrow.memory;
import java.util.ArrayDeque;
import java.util.Deque;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+
/**
* Captures details of allocation for each accountant in the hierarchical
chain.
*/
@@ -47,7 +50,7 @@ public class AllocationOutcomeDetails {
* Get the allocator that caused the failure.
* @return the allocator that caused failure, null if there was no failure.
*/
- public BufferAllocator getFailedAllocator() {
+ public @Nullable BufferAllocator getFailedAllocator() {
Entry top = allocEntries.peekLast();
if (top != null && top.allocationFailed && (top.accountant instanceof
BufferAllocator)) {
return (BufferAllocator) top.accountant;
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java
index 2c2e93b2d7..112d36ece0 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java
@@ -33,6 +33,7 @@ import org.apache.arrow.memory.util.HistoricalLog;
import org.apache.arrow.memory.util.MemoryUtil;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.util.VisibleForTesting;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* ArrowBuf serves as a facade over underlying memory by providing
@@ -68,11 +69,11 @@ public final class ArrowBuf implements AutoCloseable {
private static final int LOG_BYTES_PER_ROW = 10;
private final long id = idGenerator.incrementAndGet();
private final ReferenceManager referenceManager;
- private final BufferManager bufferManager;
+ private final @Nullable BufferManager bufferManager;
private final long addr;
private long readerIndex;
private long writerIndex;
- private final HistoricalLog historicalLog = BaseAllocator.DEBUG ?
+ private final @Nullable HistoricalLog historicalLog = BaseAllocator.DEBUG ?
new HistoricalLog(BaseAllocator.DEBUG_LOG_LENGTH, "ArrowBuf[%d]",
id) : null;
private volatile long capacity;
@@ -84,7 +85,7 @@ public final class ArrowBuf implements AutoCloseable {
*/
public ArrowBuf(
final ReferenceManager referenceManager,
- final BufferManager bufferManager,
+ final @Nullable BufferManager bufferManager,
final long capacity,
final long memoryAddress) {
this.referenceManager = referenceManager;
@@ -93,7 +94,7 @@ public final class ArrowBuf implements AutoCloseable {
this.capacity = capacity;
this.readerIndex = 0;
this.writerIndex = 0;
- if (BaseAllocator.DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("create()");
}
}
@@ -244,7 +245,7 @@ public final class ArrowBuf implements AutoCloseable {
}
@Override
- public boolean equals(Object obj) {
+ public boolean equals(@Nullable Object obj) {
// identity equals only.
return this == obj;
}
@@ -313,7 +314,7 @@ public final class ArrowBuf implements AutoCloseable {
// check bounds
Preconditions.checkArgument(fieldLength >= 0, "expecting non-negative data
length");
if (index < 0 || index > capacity() - fieldLength) {
- if (BaseAllocator.DEBUG) {
+ if (historicalLog != null) {
historicalLog.logHistory(logger);
}
throw new IndexOutOfBoundsException(String.format(
@@ -736,7 +737,7 @@ public final class ArrowBuf implements AutoCloseable {
if (length != 0) {
// copy "length" bytes from this ArrowBuf starting at addr(index) address
// into dst byte array at dstIndex onwards
- MemoryUtil.UNSAFE.copyMemory(null, addr(index), dst,
MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, length);
+ MemoryUtil.copyMemory(null, addr(index), dst,
MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, length);
}
}
@@ -773,7 +774,7 @@ public final class ArrowBuf implements AutoCloseable {
if (length > 0) {
// copy "length" bytes from src byte array at the starting index
(srcIndex)
// into this ArrowBuf starting at address "addr(index)"
- MemoryUtil.UNSAFE.copyMemory(src, MemoryUtil.BYTE_ARRAY_BASE_OFFSET +
srcIndex, null, addr(index), length);
+ MemoryUtil.copyMemory(src, MemoryUtil.BYTE_ARRAY_BASE_OFFSET + srcIndex,
null, addr(index), length);
}
}
@@ -799,7 +800,7 @@ public final class ArrowBuf implements AutoCloseable {
// at address srcAddress into the dst ByteBuffer starting at
// address dstAddress
final long dstAddress = MemoryUtil.getByteBufferAddress(dst) +
dst.position();
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress,
dst.remaining());
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress,
dst.remaining());
// after copy, bump the next write position for the dst ByteBuffer
dst.position(dst.position() + dst.remaining());
} else if (dst.hasArray()) {
@@ -807,7 +808,7 @@ public final class ArrowBuf implements AutoCloseable {
// at address srcAddress into the dst ByteBuffer starting at
// index dstIndex
final int dstIndex = dst.arrayOffset() + dst.position();
- MemoryUtil.UNSAFE.copyMemory(
+ MemoryUtil.copyMemory(
null, srcAddress, dst.array(),
MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, dst.remaining());
// after copy, bump the next write position for the dst ByteBuffer
dst.position(dst.position() + dst.remaining());
@@ -836,14 +837,14 @@ public final class ArrowBuf implements AutoCloseable {
// copy src.remaining() bytes of data from src ByteBuffer starting at
// address srcAddress into this ArrowBuf starting at address dstAddress
final long srcAddress = MemoryUtil.getByteBufferAddress(src) +
src.position();
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress,
length);
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length);
// after copy, bump the next read position for the src ByteBuffer
src.position(src.position() + length);
} else if (src.hasArray()) {
// copy src.remaining() bytes of data from src ByteBuffer starting at
// index srcIndex into this ArrowBuf starting at address dstAddress
final int srcIndex = src.arrayOffset() + src.position();
- MemoryUtil.UNSAFE.copyMemory(
+ MemoryUtil.copyMemory(
src.array(), MemoryUtil.BYTE_ARRAY_BASE_OFFSET + srcIndex,
null, dstAddress, length);
// after copy, bump the next read position for the src ByteBuffer
src.position(src.position() + length);
@@ -896,7 +897,7 @@ public final class ArrowBuf implements AutoCloseable {
// srcAddress into this ArrowBuf at address dstAddress
final long srcAddress = MemoryUtil.getByteBufferAddress(src) + srcIndex;
final long dstAddress = addr(index);
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length);
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length);
} else {
if (srcIndex == 0 && src.capacity() == length) {
// copy the entire ByteBuffer from start to end of length
@@ -936,7 +937,7 @@ public final class ArrowBuf implements AutoCloseable {
// dstAddress
final long srcAddress = addr(index);
final long dstAddress = dst.memoryAddress() + (long) dstIndex;
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length);
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length);
}
}
@@ -966,7 +967,7 @@ public final class ArrowBuf implements AutoCloseable {
// dstAddress
final long srcAddress = src.memoryAddress() + srcIndex;
final long dstAddress = addr(index);
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length);
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length);
}
}
@@ -986,7 +987,7 @@ public final class ArrowBuf implements AutoCloseable {
checkIndex(index, length);
final long srcAddress = src.memoryAddress() + src.readerIndex;
final long dstAddress = addr(index);
- MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length);
+ MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length);
src.readerIndex(src.readerIndex + length);
}
@@ -1011,7 +1012,7 @@ public final class ArrowBuf implements AutoCloseable {
if (readBytes > 0) {
// copy readBytes length of data from the tmp byte array starting
// at srcIndex 0 into this ArrowBuf starting at address addr(index)
- MemoryUtil.UNSAFE.copyMemory(tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET,
null, addr(index), readBytes);
+ MemoryUtil.copyMemory(tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET, null,
addr(index), readBytes);
}
}
return readBytes;
@@ -1033,7 +1034,7 @@ public final class ArrowBuf implements AutoCloseable {
// copy length bytes of data from this ArrowBuf starting at
// address addr(index) into the tmp byte array starting at index 0
byte[] tmp = new byte[length];
- MemoryUtil.UNSAFE.copyMemory(null, addr(index), tmp,
MemoryUtil.BYTE_ARRAY_BASE_OFFSET, length);
+ MemoryUtil.copyMemory(null, addr(index), tmp,
MemoryUtil.BYTE_ARRAY_BASE_OFFSET, length);
// write the copied data to output stream
out.write(tmp);
}
@@ -1109,7 +1110,7 @@ public final class ArrowBuf implements AutoCloseable {
public void print(StringBuilder sb, int indent, Verbosity verbosity) {
CommonUtil.indent(sb, indent).append(toString());
- if (BaseAllocator.DEBUG && verbosity.includeHistoricalLog) {
+ if (historicalLog != null && verbosity.includeHistoricalLog) {
sb.append("\n");
historicalLog.buildHistory(sb, indent + 1, verbosity.includeStackTraces);
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java
index 9337f48b74..8779c7a343 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java
@@ -30,6 +30,10 @@ import org.apache.arrow.memory.util.AssertionUtil;
import org.apache.arrow.memory.util.CommonUtil;
import org.apache.arrow.memory.util.HistoricalLog;
import org.apache.arrow.util.Preconditions;
+import org.checkerframework.checker.initialization.qual.Initialized;
+import org.checkerframework.checker.nullness.qual.KeyFor;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
import org.immutables.value.Value;
/**
@@ -64,17 +68,17 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
// Package exposed for sharing between AllocatorManger and BaseAllocator
objects
private final String name;
private final RootAllocator root;
- private final Object DEBUG_LOCK = DEBUG ? new Object() : null;
+ private final Object DEBUG_LOCK = new Object();
private final AllocationListener listener;
- private final BaseAllocator parentAllocator;
+ private final @Nullable BaseAllocator parentAllocator;
private final Map<BaseAllocator, Object> childAllocators;
private final ArrowBuf empty;
// members used purely for debugging
- private final IdentityHashMap<BufferLedger, Object> childLedgers;
- private final IdentityHashMap<Reservation, Object> reservations;
- private final HistoricalLog historicalLog;
+ private final @Nullable IdentityHashMap<BufferLedger, @Nullable Object>
childLedgers;
+ private final @Nullable IdentityHashMap<Reservation, Object> reservations;
+ private final @Nullable HistoricalLog historicalLog;
private final RoundingPolicy roundingPolicy;
- private final AllocationManager.Factory allocationManagerFactory;
+ private final AllocationManager.@NonNull Factory allocationManagerFactory;
private volatile boolean isClosed = false; // the allocator has been closed
@@ -87,8 +91,10 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
*
* @see Config
*/
+ @SuppressWarnings({"nullness:method.invocation", "nullness:cast.unsafe"})
+ //{"call to hist(,...) not allowed on the given receiver.", "cast cannot be
statically verified"}
protected BaseAllocator(
- final BaseAllocator parentAllocator,
+ final @Nullable BaseAllocator parentAllocator,
final String name,
final Config config) throws OutOfMemoryException {
super(parentAllocator, name, config.getInitReservation(),
config.getMaxAllocation());
@@ -100,7 +106,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
this.root = parentAllocator.root;
empty = parentAllocator.empty;
} else if (this instanceof RootAllocator) {
- this.root = (RootAllocator) this;
+ this.root = (@Initialized RootAllocator) this;
empty = createEmpty();
} else {
throw new IllegalStateException("An parent allocator must either carry a
root or be the " +
@@ -131,7 +137,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
}
@Override
- public BaseAllocator getParentAllocator() {
+ public @Nullable BaseAllocator getParentAllocator() {
return parentAllocator;
}
@@ -187,7 +193,9 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
assertOpen();
if (DEBUG) {
synchronized (DEBUG_LOCK) {
- childLedgers.put(ledger, null);
+ if (childLedgers != null) {
+ childLedgers.put(ledger, null);
+ }
}
}
}
@@ -201,6 +209,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
assertOpen();
if (DEBUG) {
synchronized (DEBUG_LOCK) {
+ Preconditions.checkState(childLedgers != null, "childLedgers must not
be null");
if (!childLedgers.containsKey(ledger)) {
throw new IllegalStateException("Trying to remove a child ledger
that doesn't exist.");
}
@@ -223,7 +232,9 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
synchronized (DEBUG_LOCK) {
final Object object = childAllocators.remove(childAllocator);
if (object == null) {
- childAllocator.historicalLog.logHistory(logger);
+ if (childAllocator.historicalLog != null) {
+ childAllocator.historicalLog.logHistory(logger);
+ }
throw new IllegalStateException("Child allocator[" +
childAllocator.name +
"] not found in parent allocator[" + name + "]'s childAllocators");
}
@@ -280,12 +291,13 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
return buffer(initialRequestSize, null);
}
+ @SuppressWarnings("nullness:dereference.of.nullable")//dereference of
possibly-null reference allocationManagerFactory
private ArrowBuf createEmpty() {
return allocationManagerFactory.empty();
}
@Override
- public ArrowBuf buffer(final long initialRequestSize, BufferManager manager)
{
+ public ArrowBuf buffer(final long initialRequestSize, @Nullable
BufferManager manager) {
assertOpen();
Preconditions.checkArgument(initialRequestSize >= 0, "the requested size
must be non-negative");
@@ -332,7 +344,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
*/
private ArrowBuf bufferWithoutReservation(
final long size,
- BufferManager bufferManager) throws OutOfMemoryException {
+ @Nullable BufferManager bufferManager) throws OutOfMemoryException {
assertOpen();
final AllocationManager manager = newAllocationManager(size);
@@ -388,8 +400,10 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
if (DEBUG) {
synchronized (DEBUG_LOCK) {
childAllocators.put(childAllocator, childAllocator);
- historicalLog.recordEvent("allocator[%s] created new child
allocator[%s]", name,
- childAllocator.getName());
+ if (historicalLog != null) {
+ historicalLog.recordEvent("allocator[%s] created new child
allocator[%s]", name,
+ childAllocator.getName());
+ }
}
} else {
childAllocators.put(childAllocator, childAllocator);
@@ -439,14 +453,14 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
}
// are there outstanding buffers?
- final int allocatedCount = childLedgers.size();
+ final int allocatedCount = childLedgers != null ? childLedgers.size()
: 0;
if (allocatedCount > 0) {
throw new IllegalStateException(
String.format("Allocator[%s] closed with outstanding buffers
allocated (%d).\n%s",
name, allocatedCount, toString()));
}
- if (reservations.size() != 0) {
+ if (reservations != null && reservations.size() != 0) {
throw new IllegalStateException(
String.format("Allocator[%s] closed with outstanding reservations
(%d).\n%s", name,
reservations.size(),
@@ -486,7 +500,9 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
}
if (DEBUG) {
- historicalLog.recordEvent("closed");
+ if (historicalLog != null) {
+ historicalLog.recordEvent("closed");
+ }
logger.debug(String.format("closed allocator[%s].", name));
}
@@ -517,7 +533,9 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
}
private void hist(String noteFormat, Object... args) {
- historicalLog.recordEvent(noteFormat, args);
+ if (historicalLog != null) {
+ historicalLog.recordEvent(noteFormat, args);
+ }
}
/**
@@ -567,10 +585,14 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
childTotal += Math.max(childAllocator.getAllocatedMemory(),
childAllocator.reservation);
}
if (childTotal > getAllocatedMemory()) {
- historicalLog.logHistory(logger);
+ if (historicalLog != null) {
+ historicalLog.logHistory(logger);
+ }
logger.debug("allocator[" + name + "] child event logs BEGIN");
for (final BaseAllocator childAllocator : childSet) {
- childAllocator.historicalLog.logHistory(logger);
+ if (childAllocator.historicalLog != null) {
+ childAllocator.historicalLog.logHistory(logger);
+ }
}
logger.debug("allocator[" + name + "] child event logs END");
throw new IllegalStateException(
@@ -581,33 +603,39 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
// Furthermore, the amount I've allocated should be that plus buffers
I've allocated.
long bufferTotal = 0;
- final Set<BufferLedger> ledgerSet = childLedgers.keySet();
- for (final BufferLedger ledger : ledgerSet) {
- if (!ledger.isOwningLedger()) {
- continue;
- }
+ final Set<@KeyFor("this.childLedgers") BufferLedger> ledgerSet =
childLedgers != null ?
+ childLedgers.keySet() : null;
+ if (ledgerSet != null) {
+ for (final BufferLedger ledger : ledgerSet) {
+ if (!ledger.isOwningLedger()) {
+ continue;
+ }
- final AllocationManager am = ledger.getAllocationManager();
- /*
- * Even when shared, ArrowBufs are rewrapped, so we should never see
the same instance
- * twice.
- */
- final BaseAllocator otherOwner = buffersSeen.get(am);
- if (otherOwner != null) {
- throw new IllegalStateException("This allocator's ArrowBuf already
owned by another " +
- "allocator");
- }
- buffersSeen.put(am, this);
+ final AllocationManager am = ledger.getAllocationManager();
+ /*
+ * Even when shared, ArrowBufs are rewrapped, so we should never see
the same instance
+ * twice.
+ */
+ final BaseAllocator otherOwner = buffersSeen.get(am);
+ if (otherOwner != null) {
+ throw new IllegalStateException("This allocator's ArrowBuf already
owned by another " +
+ "allocator");
+ }
+ buffersSeen.put(am, this);
- bufferTotal += am.getSize();
+ bufferTotal += am.getSize();
+ }
}
// Preallocated space has to be accounted for
- final Set<Reservation> reservationSet = reservations.keySet();
+ final Set<@KeyFor("this.reservations") Reservation> reservationSet =
reservations != null ?
+ reservations.keySet() : null;
long reservedTotal = 0;
- for (final Reservation reservation : reservationSet) {
- if (!reservation.isUsed()) {
- reservedTotal += reservation.getSize();
+ if (reservationSet != null) {
+ for (final Reservation reservation : reservationSet) {
+ if (!reservation.isUsed()) {
+ reservedTotal += reservation.getSize();
+ }
}
}
@@ -644,9 +672,13 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
if (reservedTotal != 0) {
sb.append(String.format("reserved total : %d bytes.",
reservedTotal));
- for (final Reservation reservation : reservationSet) {
- reservation.historicalLog.buildHistory(sb, 0, true);
- sb.append('\n');
+ if (reservationSet != null) {
+ for (final Reservation reservation : reservationSet) {
+ if (reservation.historicalLog != null) {
+ reservation.historicalLog.buildHistory(sb, 0, true);
+ }
+ sb.append('\n');
+ }
}
}
@@ -689,16 +721,25 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
child.print(sb, level + 2, verbosity);
}
- CommonUtil.indent(sb, level + 1).append(String.format("ledgers: %d\n",
childLedgers.size()));
- for (BufferLedger ledger : childLedgers.keySet()) {
- ledger.print(sb, level + 2, verbosity);
+ CommonUtil.indent(sb, level + 1).append(String.format("ledgers: %d\n",
childLedgers != null ?
+ childLedgers.size() : 0));
+ if (childLedgers != null) {
+ for (BufferLedger ledger : childLedgers.keySet()) {
+ ledger.print(sb, level + 2, verbosity);
+ }
}
- final Set<Reservation> reservations = this.reservations.keySet();
- CommonUtil.indent(sb, level + 1).append(String.format("reservations:
%d\n", reservations.size()));
- for (final Reservation reservation : reservations) {
- if (verbosity.includeHistoricalLog) {
- reservation.historicalLog.buildHistory(sb, level + 3, true);
+ final Set<@KeyFor("this.reservations") Reservation> reservations =
this.reservations != null ?
+ this.reservations.keySet() : null;
+ CommonUtil.indent(sb, level + 1).append(String.format("reservations:
%d\n",
+ reservations != null ? reservations.size() : 0));
+ if (reservations != null) {
+ for (final Reservation reservation : reservations) {
+ if (verbosity.includeHistoricalLog) {
+ if (reservation.historicalLog != null) {
+ reservation.historicalLog.buildHistory(sb, level + 3, true);
+ }
+ }
}
}
@@ -706,17 +747,20 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
}
- private void dumpBuffers(final StringBuilder sb, final Set<BufferLedger>
ledgerSet) {
- for (final BufferLedger ledger : ledgerSet) {
- if (!ledger.isOwningLedger()) {
- continue;
+ private void dumpBuffers(final StringBuilder sb,
+ final @Nullable Set<@KeyFor("this.childLedgers")
BufferLedger> ledgerSet) {
+ if (ledgerSet != null) {
+ for (final BufferLedger ledger : ledgerSet) {
+ if (!ledger.isOwningLedger()) {
+ continue;
+ }
+ final AllocationManager am = ledger.getAllocationManager();
+ sb.append("UnsafeDirectLittleEndian[identityHashCode == ");
+ sb.append(Integer.toString(System.identityHashCode(am)));
+ sb.append("] size ");
+ sb.append(Long.toString(am.getSize()));
+ sb.append('\n');
}
- final AllocationManager am = ledger.getAllocationManager();
- sb.append("UnsafeDirectLittleEndian[identityHashCode == ");
- sb.append(Integer.toString(System.identityHashCode(am)));
- sb.append("] size ");
- sb.append(Long.toString(am.getSize()));
- sb.append('\n');
}
}
@@ -813,7 +857,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
*/
public class Reservation implements AllocationReservation {
- private final HistoricalLog historicalLog;
+ private final @Nullable HistoricalLog historicalLog;
private int nBytes = 0;
private boolean used = false;
private boolean closed = false;
@@ -824,13 +868,16 @@ abstract class BaseAllocator extends Accountant
implements BufferAllocator {
* <p>If {@linkplain #DEBUG} is true this will capture a historical
* log of events relevant to this Reservation.
*/
+ @SuppressWarnings("nullness:argument")//to handle null assignment on third
party dependency: System.identityHashCode
public Reservation() {
if (DEBUG) {
historicalLog = new HistoricalLog("Reservation[allocator[%s], %d]",
name, System
.identityHashCode(this));
historicalLog.recordEvent("created");
synchronized (DEBUG_LOCK) {
- reservations.put(this, this);
+ if (reservations != null) {
+ reservations.put(this, this);
+ }
}
} else {
historicalLog = null;
@@ -901,7 +948,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
if (!isClosed()) {
final Object object;
synchronized (DEBUG_LOCK) {
- object = reservations.remove(this);
+ object = reservations != null ? reservations.remove(this) : null;
}
if (object == null) {
final StringBuilder sb = new StringBuilder();
@@ -911,7 +958,9 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
System.identityHashCode(this)));
}
- historicalLog.recordEvent("closed");
+ if (historicalLog != null) {
+ historicalLog.recordEvent("closed");
+ }
}
}
@@ -928,7 +977,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
final AllocationOutcome outcome =
BaseAllocator.this.allocateBytes(nBytes);
- if (DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("reserve(%d) => %s", nBytes,
Boolean.toString(outcome.isOk()));
}
@@ -959,7 +1008,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
final ArrowBuf arrowBuf =
BaseAllocator.this.bufferWithoutReservation(nBytes, null);
listener.onAllocation(nBytes);
- if (DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("allocate() => %s",
String.format("ArrowBuf[%d]", arrowBuf
.getId()));
}
@@ -982,7 +1031,7 @@ abstract class BaseAllocator extends Accountant implements
BufferAllocator {
releaseBytes(nBytes);
- if (DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("releaseReservation(%d)", nBytes);
}
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java
index 90a4ef26fb..c279e18f1e 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java
@@ -21,6 +21,7 @@ import java.util.Collection;
import org.apache.arrow.memory.rounding.DefaultRoundingPolicy;
import org.apache.arrow.memory.rounding.RoundingPolicy;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Wrapper class to deal with byte buffer allocation. Ensures users only use
designated methods.
@@ -166,7 +167,7 @@ public interface BufferAllocator extends AutoCloseable {
*
* @return parent allocator
*/
- BufferAllocator getParentAllocator();
+ @Nullable BufferAllocator getParentAllocator();
/**
* Returns the set of child allocators.
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java
index 48b3e183d5..1ca3e08ecf 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java
@@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicLong;
import org.apache.arrow.memory.util.CommonUtil;
import org.apache.arrow.memory.util.HistoricalLog;
import org.apache.arrow.util.Preconditions;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* The reference manager that binds an {@link AllocationManager} to
@@ -32,7 +33,7 @@ import org.apache.arrow.util.Preconditions;
* fate (same reference count).
*/
public class BufferLedger implements ValueWithKeyIncluded<BufferAllocator>,
ReferenceManager {
- private final IdentityHashMap<ArrowBuf, Object> buffers =
+ private final @Nullable IdentityHashMap<ArrowBuf, @Nullable Object> buffers =
BaseAllocator.DEBUG ? new IdentityHashMap<>() : null;
private static final AtomicLong LEDGER_ID_GENERATOR = new AtomicLong(0);
// unique ID assigned to each ledger
@@ -43,7 +44,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
private final long lCreationTime = System.nanoTime();
private final BufferAllocator allocator;
private final AllocationManager allocationManager;
- private final HistoricalLog historicalLog =
+ private final @Nullable HistoricalLog historicalLog =
BaseAllocator.DEBUG ? new HistoricalLog(BaseAllocator.DEBUG_LOG_LENGTH,
"BufferLedger[%d]", 1) : null;
private volatile long lDestructionTime = 0;
@@ -122,7 +123,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
"ref count decrement should be greater than or equal to 1");
// decrement the ref count
final int refCnt = decrement(decrement);
- if (BaseAllocator.DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("release(%d). original value: %d",
decrement, refCnt + decrement);
}
@@ -178,7 +179,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
@Override
public void retain(int increment) {
Preconditions.checkArgument(increment > 0, "retain(%s) argument is not
positive", increment);
- if (BaseAllocator.DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("retain(%d)", increment);
}
final int originalReferenceCount = bufRefCnt.getAndAdd(increment);
@@ -233,20 +234,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
);
// logging
- if (BaseAllocator.DEBUG) {
- historicalLog.recordEvent(
- "ArrowBuf(BufferLedger, BufferAllocator[%s], " +
- "UnsafeDirectLittleEndian[identityHashCode == " +
- "%d](%s)) => ledger hc == %d",
- allocator.getName(), System.identityHashCode(derivedBuf),
derivedBuf.toString(),
- System.identityHashCode(this));
-
- synchronized (buffers) {
- buffers.put(derivedBuf, null);
- }
- }
-
- return derivedBuf;
+ return loggingArrowBufHistoricalLog(derivedBuf);
}
/**
@@ -261,7 +249,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
* @return A new ArrowBuf that shares references with all ArrowBufs
associated
* with this BufferLedger
*/
- ArrowBuf newArrowBuf(final long length, final BufferManager manager) {
+ ArrowBuf newArrowBuf(final long length, final @Nullable BufferManager
manager) {
allocator.assertOpen();
// the start virtual address of the ArrowBuf will be same as address of
memory chunk
@@ -271,13 +259,17 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
final ArrowBuf buf = new ArrowBuf(this, manager, length, startAddress);
// logging
- if (BaseAllocator.DEBUG) {
+ return loggingArrowBufHistoricalLog(buf);
+ }
+
+ private ArrowBuf loggingArrowBufHistoricalLog(ArrowBuf buf) {
+ if (historicalLog != null) {
historicalLog.recordEvent(
"ArrowBuf(BufferLedger, BufferAllocator[%s], " +
"UnsafeDirectLittleEndian[identityHashCode == " + "%d](%s)) =>
ledger hc == %d",
allocator.getName(), System.identityHashCode(buf), buf.toString(),
System.identityHashCode(this));
-
+ Preconditions.checkState(buffers != null, "IdentityHashMap of buffers
must not be null");
synchronized (buffers) {
buffers.put(buf, null);
}
@@ -306,7 +298,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
@Override
public ArrowBuf retain(final ArrowBuf srcBuffer, BufferAllocator target) {
- if (BaseAllocator.DEBUG) {
+ if (historicalLog != null) {
historicalLog.recordEvent("retain(%s)", target.getName());
}
@@ -333,45 +325,48 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
* @param targetReferenceManager The ledger to transfer ownership account to.
* @return Whether transfer fit within target ledgers limits.
*/
- boolean transferBalance(final ReferenceManager targetReferenceManager) {
+ boolean transferBalance(final @Nullable ReferenceManager
targetReferenceManager) {
Preconditions.checkArgument(targetReferenceManager != null,
- "Expecting valid target reference manager");
- final BufferAllocator targetAllocator =
targetReferenceManager.getAllocator();
- Preconditions.checkArgument(allocator.getRoot() ==
targetAllocator.getRoot(),
- "You can only transfer between two allocators that share the same
root.");
-
- allocator.assertOpen();
- targetReferenceManager.getAllocator().assertOpen();
-
- // if we're transferring to ourself, just return.
- if (targetReferenceManager == this) {
- return true;
- }
-
- // since two balance transfers out from the allocation manager could cause
incorrect
- // accounting, we need to ensure
- // that this won't happen by synchronizing on the allocation manager
instance.
- synchronized (allocationManager) {
- if (allocationManager.getOwningLedger() != this) {
- // since the calling reference manager is not the owning
- // reference manager for the underlying memory, transfer is
- // a NO-OP
+ "Expecting valid target reference manager");
+ boolean overlimit = false;
+ if (targetReferenceManager != null) {
+ final BufferAllocator targetAllocator =
targetReferenceManager.getAllocator();
+ Preconditions.checkArgument(allocator.getRoot() ==
targetAllocator.getRoot(),
+ "You can only transfer between two allocators that share the
same root.");
+
+ allocator.assertOpen();
+ targetReferenceManager.getAllocator().assertOpen();
+
+ // if we're transferring to ourself, just return.
+ if (targetReferenceManager == this) {
return true;
}
- if (BaseAllocator.DEBUG) {
- this.historicalLog.recordEvent("transferBalance(%s)",
- targetReferenceManager.getAllocator().getName());
- }
+ // since two balance transfers out from the allocation manager could
cause incorrect
+ // accounting, we need to ensure
+ // that this won't happen by synchronizing on the allocation manager
instance.
+ synchronized (allocationManager) {
+ if (allocationManager.getOwningLedger() != this) {
+ // since the calling reference manager is not the owning
+ // reference manager for the underlying memory, transfer is
+ // a NO-OP
+ return true;
+ }
- boolean overlimit =
targetAllocator.forceAllocate(allocationManager.getSize());
- allocator.releaseBytes(allocationManager.getSize());
- // since the transfer can only happen from the owning reference manager,
- // we need to set the target ref manager as the new owning ref manager
- // for the chunk of memory in allocation manager
- allocationManager.setOwningLedger((BufferLedger) targetReferenceManager);
- return overlimit;
+ if (BaseAllocator.DEBUG && this.historicalLog != null) {
+ this.historicalLog.recordEvent("transferBalance(%s)",
+ targetReferenceManager.getAllocator().getName());
+ }
+
+ overlimit = targetAllocator.forceAllocate(allocationManager.getSize());
+ allocator.releaseBytes(allocationManager.getSize());
+ // since the transfer can only happen from the owning reference
manager,
+ // we need to set the target ref manager as the new owning ref manager
+ // for the chunk of memory in allocation manager
+ allocationManager.setOwningLedger((BufferLedger)
targetReferenceManager);
+ }
}
+ return overlimit;
}
/**
@@ -501,6 +496,7 @@ public class BufferLedger implements
ValueWithKeyIncluded<BufferAllocator>, Refe
if (!BaseAllocator.DEBUG) {
sb.append("]\n");
} else {
+ Preconditions.checkArgument(buffers != null, "IdentityHashMap of buffers
must not be null");
synchronized (buffers) {
sb.append("] holds ")
.append(buffers.size())
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
index 15120c252f..d57b72ba41 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
@@ -19,6 +19,9 @@ package org.apache.arrow.memory;
import java.lang.reflect.Field;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+
/**
* A class for choosing the default allocation manager.
*/
@@ -39,7 +42,7 @@ public class DefaultAllocationManagerOption {
/**
* The default allocation manager factory.
*/
- private static AllocationManager.Factory DEFAULT_ALLOCATION_MANAGER_FACTORY
= null;
+ private static AllocationManager.@Nullable Factory
DEFAULT_ALLOCATION_MANAGER_FACTORY = null;
/**
* The allocation manager type.
@@ -61,6 +64,7 @@ public class DefaultAllocationManagerOption {
Unknown,
}
+ @SuppressWarnings("nullness:argument") //enum types valueOf are implicitly
non-null
static AllocationManagerType getDefaultAllocationManagerType() {
AllocationManagerType ret = AllocationManagerType.Unknown;
@@ -103,6 +107,9 @@ public class DefaultAllocationManagerOption {
return DEFAULT_ALLOCATION_MANAGER_FACTORY;
}
+ @SuppressWarnings({"nullness:argument", "nullness:return"})
+ //incompatible argument for parameter obj of Field.get
+ // Static member qualifying type may not be annotated
private static AllocationManager.Factory getFactory(String clazzName) {
try {
Field field = Class.forName(clazzName).getDeclaredField("FACTORY");
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java
index edfa82392a..740233ef41 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java
@@ -19,6 +19,9 @@ package org.apache.arrow.memory;
import org.apache.arrow.util.Preconditions;
import org.apache.arrow.util.VisibleForTesting;
+import org.checkerframework.checker.initialization.qual.Initialized;
+import org.checkerframework.checker.initialization.qual.UnderInitialization;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Highly specialized IdentityHashMap that implements only partial
@@ -35,7 +38,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
/*
* The internal data structure to hold values.
*/
- private Object[] elementData;
+ private @Nullable Object [] elementData; // elementData[index] = null;
/* Actual number of values. */
private int size;
@@ -69,19 +72,20 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
if (maxSize >= 0) {
this.size = 0;
threshold = getThreshold(maxSize);
- elementData = newElementArray(computeElementArraySize());
+ elementData = newElementArrayUnderInitialized(computeElementArraySize());
} else {
throw new IllegalArgumentException();
}
}
- private int getThreshold(int maxSize) {
+ private int getThreshold(@UnderInitialization LowCostIdentityHashMap<K, V>
this,
+ int maxSize) {
// assign the threshold to maxSize initially, this will change to a
// higher value if rehashing occurs.
return maxSize > 2 ? maxSize : 2;
}
- private int computeElementArraySize() {
+ private int computeElementArraySize(@UnderInitialization
LowCostIdentityHashMap<K, V> this) {
int arraySize = (int) (((long) threshold * 10000) / LOAD_FACTOR);
// ensure arraySize is positive, the above cast from long to int type
// leads to overflow and negative arraySize if threshold is too big
@@ -95,7 +99,18 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
* the number of elements
* @return Reference to the element array
*/
- private Object[] newElementArray(int s) {
+ private Object[] newElementArrayInitialized(@Initialized
LowCostIdentityHashMap<K, V> this, int s) {
+ return new Object[s];
+ }
+
+ /**
+ * Create a new element array.
+ *
+ * @param s
+ * the number of elements
+ * @return Reference to the element array
+ */
+ private Object[] newElementArrayUnderInitialized(@UnderInitialization
LowCostIdentityHashMap<K, V> this, int s) {
return new Object[s];
}
@@ -152,7 +167,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
* @param key the key.
* @return the value of the mapping with the specified key.
*/
- public V get(K key) {
+ public @Nullable V get(K key) {
Preconditions.checkNotNull(key);
int index = findIndex(key, elementData);
@@ -166,7 +181,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
* empty spot if the key is not found in this table.
*/
@VisibleForTesting
- int findIndex(Object key, Object[] array) {
+ int findIndex(@Nullable Object key, @Nullable Object[] array) {
int length = array.length;
int index = getModuloHash(key, length);
int last = (index + length - 1) % length;
@@ -184,7 +199,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
}
@VisibleForTesting
- static int getModuloHash(Object key, int length) {
+ static int getModuloHash(@Nullable Object key, int length) {
return ((System.identityHashCode(key) & 0x7FFFFFFF) % length);
}
@@ -226,7 +241,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
if (newlength == 0) {
newlength = 1;
}
- Object[] newData = newElementArray(newlength);
+ @Nullable Object[] newData = newElementArrayInitialized(newlength);
for (int i = 0; i < elementData.length; i++) {
Object key = (elementData[i] == null) ? null : ((V)
elementData[i]).getKey();
if (key != null) {
@@ -250,7 +265,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
* @return the value of the removed mapping, or {@code null} if no mapping
* for the specified key was found.
*/
- public V remove(K key) {
+ public @Nullable V remove(K key) {
Preconditions.checkNotNull(key);
boolean hashedOk;
@@ -325,7 +340,7 @@ public class LowCostIdentityHashMap<K, V extends
ValueWithKeyIncluded<K>> {
*
* @return next available value or null if none available
*/
- public V getNextValue() {
+ public @Nullable V getNextValue() {
for (int i = 0; i < elementData.length; i++) {
if (elementData[i] != null) {
return (V) elementData[i];
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java
index fa1cfbdb29..b41576847d 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java
@@ -21,6 +21,7 @@ import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.util.hash.ArrowBufHasher;
import org.apache.arrow.memory.util.hash.SimpleHasher;
import org.apache.arrow.util.Preconditions;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Pointer to a memory region within an {@link ArrowBuf}.
@@ -33,7 +34,7 @@ public final class ArrowBufPointer {
*/
public static final int NULL_HASH_CODE = 0;
- private ArrowBuf buf;
+ private @Nullable ArrowBuf buf;
private long offset;
@@ -62,6 +63,7 @@ public final class ArrowBufPointer {
public ArrowBufPointer(ArrowBufHasher hasher) {
Preconditions.checkNotNull(hasher);
this.hasher = hasher;
+ this.buf = null;
}
/**
@@ -93,6 +95,7 @@ public final class ArrowBufPointer {
* @param offset the start off set of the memory region pointed to.
* @param length the length off set of the memory region pointed to.
*/
+
public void set(ArrowBuf buf, long offset, long length) {
this.buf = buf;
this.offset = offset;
@@ -105,7 +108,7 @@ public final class ArrowBufPointer {
* Gets the underlying buffer, or null if the underlying data is invalid or
null.
* @return the underlying buffer, if any, or null if the underlying data is
invalid or null.
*/
- public ArrowBuf getBuf() {
+ public @Nullable ArrowBuf getBuf() {
return buf;
}
@@ -118,7 +121,7 @@ public final class ArrowBufPointer {
}
@Override
- public boolean equals(Object o) {
+ public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java
index f02539a8a3..21f063c939 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java
@@ -20,6 +20,7 @@ package org.apache.arrow.memory.util;
import java.util.Arrays;
import java.util.LinkedList;
+import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
/**
@@ -32,7 +33,7 @@ public class HistoricalLog {
private final LinkedList<Event> history = new LinkedList<>();
private final String idString; // the formatted id string
private final int limit; // the limit on the number of events kept
- private Event firstEvent; // the first stack trace recorded
+ private @Nullable Event firstEvent; // the first stack trace recorded
/**
* Constructor. The format string will be formatted and have its arguments
@@ -68,6 +69,7 @@ public class HistoricalLog {
public HistoricalLog(final int limit, final String idStringFormat, Object...
args) {
this.limit = limit;
this.idString = String.format(idStringFormat, args);
+ this.firstEvent = null;
}
/**
@@ -122,13 +124,16 @@ public class HistoricalLog {
.append('\n');
if (firstEvent != null) {
+ long time = firstEvent.time;
+ String note = firstEvent.note;
+ final StackTrace stackTrace = firstEvent.stackTrace;
sb.append(innerIndentation)
- .append(firstEvent.time)
+ .append(time)
.append(' ')
- .append(firstEvent.note)
+ .append(note)
.append('\n');
if (includeStackTrace) {
- firstEvent.stackTrace.writeToBuilder(sb, indent + 2);
+ stackTrace.writeToBuilder(sb, indent + 2);
}
for (final Event event : history) {
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
index cc615c5b38..f79cf79531 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
@@ -25,15 +25,18 @@ import java.nio.ByteOrder;
import java.security.AccessController;
import java.security.PrivilegedAction;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
import sun.misc.Unsafe;
+
/**
* Utilities for memory related operations.
*/
public class MemoryUtil {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(MemoryUtil.class);
- private static final Constructor<?> DIRECT_BUFFER_CONSTRUCTOR;
+ private static final @Nullable Constructor<?> DIRECT_BUFFER_CONSTRUCTOR;
/**
* The unsafe object from which to access the off-heap memory.
*/
@@ -63,6 +66,9 @@ public class MemoryUtil {
// try to get the unsafe object
final Object maybeUnsafe = AccessController.doPrivileged(new
PrivilegedAction<Object>() {
@Override
+ @SuppressWarnings({"nullness:argument", "nullness:return"})
+ // incompatible argument for parameter obj of Field.get
+ // incompatible types in return
public Object run() {
try {
final Field unsafeField =
Unsafe.class.getDeclaredField("theUnsafe");
@@ -179,4 +185,11 @@ public class MemoryUtil {
throw new UnsupportedOperationException(
"sun.misc.Unsafe or java.nio.DirectByteBuffer.<init>(long, int) not
available");
}
+
+ @SuppressWarnings("nullness:argument") //to handle null assignment on third
party dependency: Unsafe
+ public static void copyMemory(@Nullable Object srcBase, long srcOffset,
+ @Nullable Object destBase, long destOffset,
+ long bytes) {
+ UNSAFE.copyMemory(srcBase, srcOffset, destBase, destOffset, bytes);
+ }
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java
index cd864eb998..a533edd793 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java
@@ -19,12 +19,15 @@ package org.apache.arrow.memory.util;
import java.util.Arrays;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+
/**
* Convenient way of obtaining and manipulating stack traces for debugging.
*/
public class StackTrace {
- private final StackTraceElement[] stackTraceElements;
+ private final @Nullable StackTraceElement [] stackTraceElements;
/**
* Constructor. Captures the current stack trace.
@@ -48,16 +51,18 @@ public class StackTrace {
// write the stack trace in standard Java format
for (StackTraceElement ste : stackTraceElements) {
- sb.append(indentation)
- .append("at ")
- .append(ste.getClassName())
- .append('.')
- .append(ste.getMethodName())
- .append('(')
- .append(ste.getFileName())
- .append(':')
- .append(Integer.toString(ste.getLineNumber()))
- .append(")\n");
+ if (ste != null) {
+ sb.append(indentation)
+ .append("at ")
+ .append(ste.getClassName())
+ .append('.')
+ .append(ste.getMethodName())
+ .append('(')
+ .append(ste.getFileName())
+ .append(':')
+ .append(Integer.toString(ste.getLineNumber()))
+ .append(")\n");
+ }
}
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java
index 75fc3f0c45..5de98d23bb 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java
@@ -19,6 +19,7 @@ package org.apache.arrow.memory.util.hash;
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.util.MemoryUtil;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* Implementation of the Murmur hashing algorithm.
@@ -157,7 +158,7 @@ public class MurmurHasher implements ArrowBufHasher {
}
@Override
- public boolean equals(Object o) {
+ public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java
index da0ee48299..3bf3c2a828 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java
@@ -20,6 +20,7 @@ package org.apache.arrow.memory.util.hash;
import org.apache.arrow.memory.ArrowBuf;
import org.apache.arrow.memory.util.MemoryUtil;
+import org.checkerframework.checker.nullness.qual.Nullable;
/**
* A simple hasher that calculates the hash code of integers as is,
@@ -110,7 +111,7 @@ public class SimpleHasher implements ArrowBufHasher {
}
@Override
- public boolean equals(Object obj) {
+ public boolean equals(@Nullable Object obj) {
return obj != null && (obj instanceof SimpleHasher);
}
}
diff --git
a/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java
b/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java
index 0ffc9447e4..8083033007 100644
---
a/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java
+++
b/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java
@@ -33,6 +33,8 @@
package org.apache.arrow.util;
+import org.checkerframework.dataflow.qual.AssertMethod;
+
/**
* Static convenience methods that help a method or constructor check whether
it was invoked
* correctly (whether its <i>preconditions</i> have been met). These methods
generally accept a
@@ -117,6 +119,7 @@ public final class Preconditions {
* @param expression a boolean expression
* @throws IllegalArgumentException if {@code expression} is false
*/
+ @AssertMethod
public static void checkArgument(boolean expression) {
if (!expression) {
throw new IllegalArgumentException();
@@ -131,6 +134,7 @@ public final class Preconditions {
* string using {@link String#valueOf(Object)}
* @throws IllegalArgumentException if {@code expression} is false
*/
+ @AssertMethod
public static void checkArgument(boolean expression, Object errorMessage) {
if (!expression) {
throw new IllegalArgumentException(String.valueOf(errorMessage));
@@ -438,6 +442,7 @@ public final class Preconditions {
* @param expression a boolean expression
* @throws IllegalStateException if {@code expression} is false
*/
+ @AssertMethod
public static void checkState(boolean expression) {
if (!expression) {
throw new IllegalStateException();
@@ -453,6 +458,7 @@ public final class Preconditions {
* string using {@link String#valueOf(Object)}
* @throws IllegalStateException if {@code expression} is false
*/
+ @AssertMethod
public static void checkState(boolean expression, Object errorMessage) {
if (!expression) {
throw new IllegalStateException(String.valueOf(errorMessage));
diff --git a/java/pom.xml b/java/pom.xml
index 6b7192fd33..62e63d41a9 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -48,6 +48,7 @@
<maven-compiler-plugin.version>3.11.0</maven-compiler-plugin.version>
<mockito.core.version>5.5.0</mockito.core.version>
<mockito.inline.version>5.2.0</mockito.inline.version>
+ <checker.framework.version>3.42.0</checker.framework.version>
</properties>
<scm>
@@ -354,6 +355,7 @@
<!-- source annotations (not kept in compiled code) -->
<ignoredDependency>javax.annotation:javax.annotation-api:*</ignoredDependency>
<ignoredDependency>org.apache.hadoop:hadoop-client-api</ignoredDependency>
+
<ignoredDependency>org.checkerframework:checker-qual</ignoredDependency>
</ignoredDependencies>
</configuration>
</execution>
@@ -606,6 +608,11 @@
<type>pom</type>
<scope>import</scope>
</dependency>
+ <dependency>
+ <groupId>org.checkerframework</groupId>
+ <artifactId>checker-qual</artifactId>
+ <version>${checker.framework.version}</version>
+ </dependency>
<dependency>
<groupId>com.google.flatbuffers</groupId>
<artifactId>flatbuffers-java</artifactId>
