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

    https://github.com/apache/spark/pull/20676#discussion_r171116974
  
    --- Diff: 
core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala ---
    @@ -246,18 +246,18 @@ private[spark] class MemoryStore(
             val amountToRequest = size - unrollMemoryUsedByThisBlock
             keepUnrolling = reserveUnrollMemoryForThisTask(blockId, 
amountToRequest, memoryMode)
             if (keepUnrolling) {
    -          unrollMemoryUsedByThisBlock += amountToRequest
    +          unrollMemoryUsedByThisBlock = size
             }
    +      } else if (size < unrollMemoryUsedByThisBlock) {
    +        releaseUnrollMemoryForThisTask(memoryMode, 
unrollMemoryUsedByThisBlock - size)
    +        unrollMemoryUsedByThisBlock = size
           }
     
           if (keepUnrolling) {
             val entry = entryBuilder.build()
    -        // Synchronize so that transfer is atomic
    -        memoryManager.synchronized {
    -          releaseUnrollMemoryForThisTask(memoryMode, 
unrollMemoryUsedByThisBlock)
    -          val success = memoryManager.acquireStorageMemory(blockId, 
entry.size, memoryMode)
    -          assert(success, "transferring unroll memory to storage memory 
failed")
    -        }
    +        // In fact, unroll memory is also storage memory, it is 
unnecessary to
    +        // release unroll memory really
    +        releaseUnrollMemoryForThisTask(memoryMode, 
unrollMemoryUsedByThisBlock, false)
    --- End diff --
    
     t am sure all other calls' third param default to be true


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to