You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by gu...@apache.org on 2018/12/15 05:52:19 UTC
[spark] branch master updated: [SPARK-26265][CORE][FOLLOWUP] Put
freePage into a finally block
This is an automated email from the ASF dual-hosted git repository.
gurwls223 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 1b604c1 [SPARK-26265][CORE][FOLLOWUP] Put freePage into a finally block
1b604c1 is described below
commit 1b604c1fd0b9ef17b394818fbd6c546bc01cdd8c
Author: Liang-Chi Hsieh <vi...@gmail.com>
AuthorDate: Sat Dec 15 13:52:07 2018 +0800
[SPARK-26265][CORE][FOLLOWUP] Put freePage into a finally block
## What changes were proposed in this pull request?
Based on the [comment](https://github.com/apache/spark/pull/23272#discussion_r240735509), it seems to be better to put `freePage` into a `finally` block. This patch as a follow-up to do so.
## How was this patch tested?
Existing tests.
Closes #23294 from viirya/SPARK-26265-followup.
Authored-by: Liang-Chi Hsieh <vi...@gmail.com>
Signed-off-by: Hyukjin Kwon <gu...@apache.org>
---
.../apache/spark/unsafe/map/BytesToBytesMap.java | 57 ++++++++++++----------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java b/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
index fbba002..7df8aaf 100644
--- a/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
+++ b/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
@@ -262,36 +262,39 @@ public final class BytesToBytesMap extends MemoryConsumer {
// reference to the page to free and free it after releasing the lock of `MapIterator`.
MemoryBlock pageToFree = null;
- synchronized (this) {
- int nextIdx = dataPages.indexOf(currentPage) + 1;
- if (destructive && currentPage != null) {
- dataPages.remove(currentPage);
- pageToFree = currentPage;
- nextIdx --;
- }
- if (dataPages.size() > nextIdx) {
- currentPage = dataPages.get(nextIdx);
- pageBaseObject = currentPage.getBaseObject();
- offsetInPage = currentPage.getBaseOffset();
- recordsInPage = UnsafeAlignedOffset.getSize(pageBaseObject, offsetInPage);
- offsetInPage += UnsafeAlignedOffset.getUaoSize();
- } else {
- currentPage = null;
- if (reader != null) {
- handleFailedDelete();
+ try {
+ synchronized (this) {
+ int nextIdx = dataPages.indexOf(currentPage) + 1;
+ if (destructive && currentPage != null) {
+ dataPages.remove(currentPage);
+ pageToFree = currentPage;
+ nextIdx--;
}
- try {
- Closeables.close(reader, /* swallowIOException = */ false);
- reader = spillWriters.getFirst().getReader(serializerManager);
- recordsInPage = -1;
- } catch (IOException e) {
- // Scala iterator does not handle exception
- Platform.throwException(e);
+ if (dataPages.size() > nextIdx) {
+ currentPage = dataPages.get(nextIdx);
+ pageBaseObject = currentPage.getBaseObject();
+ offsetInPage = currentPage.getBaseOffset();
+ recordsInPage = UnsafeAlignedOffset.getSize(pageBaseObject, offsetInPage);
+ offsetInPage += UnsafeAlignedOffset.getUaoSize();
+ } else {
+ currentPage = null;
+ if (reader != null) {
+ handleFailedDelete();
+ }
+ try {
+ Closeables.close(reader, /* swallowIOException = */ false);
+ reader = spillWriters.getFirst().getReader(serializerManager);
+ recordsInPage = -1;
+ } catch (IOException e) {
+ // Scala iterator does not handle exception
+ Platform.throwException(e);
+ }
}
}
- }
- if (pageToFree != null) {
- freePage(pageToFree);
+ } finally {
+ if (pageToFree != null) {
+ freePage(pageToFree);
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org