You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ra...@apache.org on 2017/08/30 11:16:16 UTC
carbondata git commit: [CARBONDATA-1393] Avoid to throw NPE when
execute 'freeMemory' of UnsafeMemoryManager/UnsafeSortMemoryManager
Repository: carbondata
Updated Branches:
refs/heads/master 8ea7272d9 -> 289232607
[CARBONDATA-1393] Avoid to throw NPE when execute 'freeMemory' of UnsafeMemoryManager/UnsafeSortMemoryManager
UnsafeMemoryManager.freeMemoryAll(long taskId) may run before freeMemory(long taskId, MemoryBlock memoryBlock), so taskIdToMemoryBlockMap.get(taskId) will return null and then throw NPE.
This closes #1266
Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo
Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/28923260
Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/28923260
Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/28923260
Branch: refs/heads/master
Commit: 289232607ae51f350c0dc13dcbb9da7b95491c11
Parents: 8ea7272
Author: Zhang Zhichao <44...@qq.com>
Authored: Fri Aug 18 17:04:50 2017 +0800
Committer: Ravindra Pesala <ra...@gmail.com>
Committed: Wed Aug 30 16:45:52 2017 +0530
----------------------------------------------------------------------
.../core/memory/HeapMemoryAllocator.java | 3 +++
.../carbondata/core/memory/MemoryBlock.java | 14 +++++++++++
.../core/memory/UnsafeMemoryAllocator.java | 1 +
.../core/memory/UnsafeMemoryManager.java | 24 +++++++++++-------
.../core/memory/UnsafeSortMemoryManager.java | 26 ++++++++++++--------
5 files changed, 49 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java b/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
index 2b63726..5862933 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
@@ -53,6 +53,8 @@ public class HeapMemoryAllocator implements MemoryAllocator {
final MemoryBlock memory = blockReference.get();
if (memory != null) {
assert (memory.size() == size);
+ // reuse this MemoryBlock
+ memory.setFreedStatus(false);
return memory;
}
}
@@ -76,5 +78,6 @@ public class HeapMemoryAllocator implements MemoryAllocator {
pool.add(new WeakReference<>(memory));
}
}
+ memory.setFreedStatus(true);
}
}
http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java b/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
index 74136c1..d6cb184 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
@@ -29,9 +29,15 @@ public class MemoryBlock extends MemoryLocation {
private final long length;
+ /**
+ * whether freed or not
+ */
+ private boolean isFreed = false;
+
public MemoryBlock(@Nullable Object obj, long offset, long length) {
super(obj, offset);
this.length = length;
+ this.isFreed = false;
}
/**
@@ -41,6 +47,14 @@ public class MemoryBlock extends MemoryLocation {
return length;
}
+ public boolean isFreedStatus() {
+ return this.isFreed;
+ }
+
+ public void setFreedStatus(boolean freedStatus) {
+ this.isFreed = freedStatus;
+ }
+
/**
* Creates a memory block pointing to the memory used by the long array.
*/
http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
index fcb0b88..67412ac 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
@@ -36,5 +36,6 @@ public class UnsafeMemoryAllocator implements MemoryAllocator {
assert (memory.obj == null) :
"baseObject not null; are you trying to use the off-heap allocator to free on-heap memory?";
CarbonUnsafe.getUnsafe().freeMemory(memory.offset);
+ memory.setFreedStatus(true);
}
}
http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
index 1190d56..06f907d 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
@@ -105,13 +105,17 @@ public class UnsafeMemoryManager {
}
public synchronized void freeMemory(long taskId, MemoryBlock memoryBlock) {
- taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
- allocator.free(memoryBlock);
- memoryUsed -= memoryBlock.size();
- memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
- LOGGER.info(
- "Freeing memory of size: " + memoryBlock.size() + "available memory: " + (totalMemory
- - memoryUsed));
+ if (taskIdToMemoryBlockMap.containsKey(taskId)) {
+ taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
+ }
+ if (!memoryBlock.isFreedStatus()) {
+ allocator.free(memoryBlock);
+ memoryUsed -= memoryBlock.size();
+ memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
+ LOGGER.info(
+ "Freeing memory of size: " + memoryBlock.size() + "available memory: " + (totalMemory
+ - memoryUsed));
+ }
}
public synchronized void freeMemoryAll(long taskId) {
@@ -123,8 +127,10 @@ public class UnsafeMemoryManager {
MemoryBlock memoryBlock = null;
while (iterator.hasNext()) {
memoryBlock = iterator.next();
- occuppiedMemory += memoryBlock.size();
- allocator.free(memoryBlock);
+ if (!memoryBlock.isFreedStatus()) {
+ occuppiedMemory += memoryBlock.size();
+ allocator.free(memoryBlock);
+ }
}
}
memoryUsed -= occuppiedMemory;
http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
index b4a200a..c63b320 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
@@ -135,14 +135,18 @@ public class UnsafeSortMemoryManager {
}
public synchronized void freeMemory(long taskId, MemoryBlock memoryBlock) {
- allocator.free(memoryBlock);
- taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
- memoryUsed -= memoryBlock.size();
- memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
- if (LOGGER.isDebugEnabled()) {
- LOGGER.debug(
- "Freeing memory of size: " + memoryBlock.size() + ": Current available memory is: " + (
- totalMemory - memoryUsed));
+ if (taskIdToMemoryBlockMap.containsKey(taskId)) {
+ taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
+ }
+ if (!memoryBlock.isFreedStatus()) {
+ allocator.free(memoryBlock);
+ memoryUsed -= memoryBlock.size();
+ memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug(
+ "Freeing memory of size: " + memoryBlock.size() + ": Current available memory is: " + (
+ totalMemory - memoryUsed));
+ }
}
}
@@ -161,8 +165,10 @@ public class UnsafeSortMemoryManager {
MemoryBlock memoryBlock = null;
while (iterator.hasNext()) {
memoryBlock = iterator.next();
- occuppiedMemory += memoryBlock.size();
- allocator.free(memoryBlock);
+ if (!memoryBlock.isFreedStatus()) {
+ occuppiedMemory += memoryBlock.size();
+ allocator.free(memoryBlock);
+ }
}
}
memoryUsed -= occuppiedMemory;