You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2017/09/29 13:58:15 UTC
arrow git commit: ARROW-1618: [JAVA] Reduce Heap Usage (Phase 1)
Repository: arrow
Updated Branches:
refs/heads/master bcb29d081 -> 732367766
ARROW-1618: [JAVA] Reduce Heap Usage (Phase 1)
cc @jacques-n , @icexelloss , @BryanCutler
This is initial small phase of our attempt to reduce heap usage per vector.
As part of investigation, we realized its better to address few things as part of subtasks for ARROW-1463. Accordingly, I need to update the requirements document w.r.t heap usage for ARROW-1471.
This patch gets rid of Release Listener object in Allocation Manager as all the logic is implemented as part of AllocationManager itself.
https://docs.google.com/document/d/1MU-ah_bBHIxXNrd7SkwewGCOOexkXJ7cgKaCis5f-PI/edit
Author: siddharth <si...@dremio.com>
Closes #1142 from siddharthteotia/ARROW-1618 and squashes the following commits:
77151a27 [siddharth] ARROW-1618: Reduce Heap Usage (Phase 1)
Project: http://git-wip-us.apache.org/repos/asf/arrow/repo
Commit: http://git-wip-us.apache.org/repos/asf/arrow/commit/73236776
Tree: http://git-wip-us.apache.org/repos/asf/arrow/tree/73236776
Diff: http://git-wip-us.apache.org/repos/asf/arrow/diff/73236776
Branch: refs/heads/master
Commit: 7323677665c5b71203a1e526c0a19eb5520442d0
Parents: bcb29d0
Author: siddharth <si...@dremio.com>
Authored: Fri Sep 29 08:58:09 2017 -0500
Committer: Wes McKinney <we...@twosigma.com>
Committed: Fri Sep 29 08:58:09 2017 -0500
----------------------------------------------------------------------
.../apache/arrow/memory/AllocationManager.java | 81 +++++++++-----------
1 file changed, 37 insertions(+), 44 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/arrow/blob/73236776/java/memory/src/main/java/org/apache/arrow/memory/AllocationManager.java
----------------------------------------------------------------------
diff --git a/java/memory/src/main/java/org/apache/arrow/memory/AllocationManager.java b/java/memory/src/main/java/org/apache/arrow/memory/AllocationManager.java
index c528937..6877c18 100644
--- a/java/memory/src/main/java/org/apache/arrow/memory/AllocationManager.java
+++ b/java/memory/src/main/java/org/apache/arrow/memory/AllocationManager.java
@@ -140,7 +140,7 @@ public class AllocationManager {
return existingLedger;
}
- final BufferLedger ledger = new BufferLedger(allocator, new ReleaseListener(allocator));
+ final BufferLedger ledger = new BufferLedger(allocator);
if (retain) {
ledger.inc();
}
@@ -151,54 +151,41 @@ public class AllocationManager {
}
}
-
/**
* The way that a particular BufferLedger communicates back to the AllocationManager that it
* now longer needs to hold
* a reference to particular piece of memory.
+ * Can only be called when you already hold the writeLock.
*/
- private class ReleaseListener {
-
- private final BufferAllocator allocator;
-
- public ReleaseListener(BufferAllocator allocator) {
- this.allocator = allocator;
- }
-
- /**
- * Can only be called when you already hold the writeLock.
- */
- public void release() {
- allocator.assertOpen();
+ private void release(final BufferLedger ledger) {
+ final BaseAllocator allocator = ledger.getAllocator();
+ allocator.assertOpen();
- final BufferLedger oldLedger = map.remove(allocator);
- oldLedger.allocator.dissociateLedger(oldLedger);
+ final BufferLedger oldLedger = map.remove(allocator);
+ oldLedger.allocator.dissociateLedger(oldLedger);
- if (oldLedger == owningLedger) {
- if (map.isEmpty()) {
- // no one else owns, lets release.
- oldLedger.allocator.releaseBytes(size);
- underlying.release();
- amDestructionTime = System.nanoTime();
- owningLedger = null;
- } else {
- // we need to change the owning allocator. we've been removed so we'll get whatever is
- // top of list
- BufferLedger newLedger = map.values().iterator().next();
-
- // we'll forcefully transfer the ownership and not worry about whether we exceeded the
- // limit
- // since this consumer can't do anything with this.
- oldLedger.transferBalance(newLedger);
- }
+ if (oldLedger == owningLedger) {
+ if (map.isEmpty()) {
+ // no one else owns, lets release.
+ oldLedger.allocator.releaseBytes(size);
+ underlying.release();
+ amDestructionTime = System.nanoTime();
+ owningLedger = null;
} else {
- if (map.isEmpty()) {
- throw new IllegalStateException("The final removal of a ledger should be connected to " +
- "the owning ledger.");
- }
+ // we need to change the owning allocator. we've been removed so we'll get whatever is
+ // top of list
+ BufferLedger newLedger = map.values().iterator().next();
+
+ // we'll forcefully transfer the ownership and not worry about whether we exceeded the
+ // limit
+ // since this consumer can't do anything with this.
+ oldLedger.transferBalance(newLedger);
+ }
+ } else {
+ if (map.isEmpty()) {
+ throw new IllegalStateException("The final removal of a ledger should be connected to " +
+ "the owning ledger.");
}
-
-
}
}
@@ -221,16 +208,22 @@ public class AllocationManager {
// correctly
private final long lCreationTime = System.nanoTime();
private final BaseAllocator allocator;
- private final ReleaseListener listener;
private final HistoricalLog historicalLog = BaseAllocator.DEBUG ? new HistoricalLog
(BaseAllocator.DEBUG_LOG_LENGTH,
"BufferLedger[%d]", 1)
: null;
private volatile long lDestructionTime = 0;
- private BufferLedger(BaseAllocator allocator, ReleaseListener listener) {
+ private BufferLedger(BaseAllocator allocator) {
this.allocator = allocator;
- this.listener = listener;
+ }
+
+ /**
+ * Get the allocator for this ledger
+ * @return allocator
+ */
+ private BaseAllocator getAllocator() {
+ return allocator;
}
/**
@@ -340,7 +333,7 @@ public class AllocationManager {
outcome = bufRefCnt.addAndGet(-decrement);
if (outcome == 0) {
lDestructionTime = System.nanoTime();
- listener.release();
+ release(this);
}
}