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 <[email protected]>
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 <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
---
.../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: [email protected]
For additional commands, e-mail: [email protected]
