lidavidm commented on code in PR #14506:
URL: https://github.com/apache/arrow/pull/14506#discussion_r1007055553
##########
java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java:
##########
@@ -22,43 +22,35 @@
import org.apache.arrow.util.Preconditions;
/**
- * The abstract base class of AllocationManager.
+ * An AllocationManager is the implementation of a physical memory allocation.
*
- * <p>Manages the relationship between one or more allocators and a particular
UDLE. Ensures that
- * one allocator owns the
- * memory that multiple allocators may be referencing. Manages a BufferLedger
between each of its
- * associated allocators.
+ * <p>Manages the relationship between the allocators and a particular memory
allocation. Ensures that
+ * one allocator owns the memory that multiple allocators may be referencing.
Manages a BufferLedger between
+ * each of its associated allocators. It does not track the reference count;
that is the role of {@link BufferLedger}
+ * (aka {@link ReferenceManager}).
*
- * <p>The only reason that this isn't package private is we're forced to put
ArrowBuf in Netty's
- * package which need access
- * to these objects or methods.
+ * <p>This is a public interface implemented by concrete allocator
implementations (e.g. Netty or Unsafe).
*
* <p>Threading: AllocationManager manages thread-safety internally.
Operations within the context
- * of a single BufferLedger
- * are lockless in nature and can be leveraged by multiple threads. Operations
that cross the
- * context of two ledgers
- * will acquire a lock on the AllocationManager instance. Important note,
there is one
- * AllocationManager per
- * UnsafeDirectLittleEndian buffer allocation. As such, there will be
thousands of these in a
- * typical query. The
- * contention of acquiring a lock on AllocationManager should be very low.
+ * of a single BufferLedger are lockless in nature and can be leveraged by
multiple threads. Operations that cross the
+ * context of two ledgers will acquire a lock on the AllocationManager
instance. Important note, there is one
+ * AllocationManager per physical buffer allocation. As such, there will be
thousands of these in a
+ * typical query. The contention of acquiring a lock on AllocationManager
should be very low.
*/
public abstract class AllocationManager {
-
- private static final AtomicLong MANAGER_ID_GENERATOR = new AtomicLong(0);
-
+ // The RootAllocator we are associated with. An allocation can only ever be
associated with a single RootAllocator.
private final BufferAllocator root;
- private final long allocatorManagerId =
MANAGER_ID_GENERATOR.incrementAndGet();
- // ARROW-1627 Trying to minimize memory overhead caused by previously used
IdentityHashMap
- // see JIRA for details
+ // An allocation can be tracked by multiple allocators. (This is because an
allocator is more like a ledger.)
+ // All such allocators track reference counts individually, via BufferLedger
instances. When an individual
+ // reference count reaches zero, the allocator will be dissociated from this
allocation. If that was via the
+ // owningLedger, then no more allocators should be tracking this allocation,
and the allocation will be freed.
+ // ARROW-1627: Trying to minimize memory overhead caused by previously used
IdentityHashMap
private final LowCostIdentityHashMap<BufferAllocator, BufferLedger> map =
new LowCostIdentityHashMap<>();
- private final long amCreationTime = System.nanoTime();
-
- // The ReferenceManager created at the time of creation of this
AllocationManager
- // is treated as the owning reference manager for the underlying chunk of
memory
- // managed by this allocation manager
+ // The primary BufferLedger (i.e. reference count) tracking this allocation.
+ // 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.
Review Comment:
I meant extant here: `Still in existence; not destroyed, lost, or extinct.`
--
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
