Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8805#discussion_r39885230
  
    --- Diff: 
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorter.java
 ---
    @@ -254,22 +281,25 @@ public int getNumberOfAllocatedPages() {
     
       /**
        * Free this sorter's in-memory data structures, including its data 
pages and pointer array.
    -   *
    +   * @param bytesToReserve number of bytes to hold onto when releasing 
memory for this task.
        * @return the number of bytes freed.
        */
    -  private long freeMemory() {
    +  private long freeMemory(long bytesToReserve) {
         updatePeakMemoryUsed();
         long memoryFreed = 0;
    +    long remainingBytesToReserve = bytesToReserve;
         for (MemoryBlock block : allocatedPages) {
    --- End diff --
    
    > Do we really have to release all of the data pages? 
    
    What do you mean? That's exactly what this patch avoids. If we know we need 
a page immediately after the spill, we should release `n - 1` pages, where `n` 
is the number of pages we currently hold.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to