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

    https://github.com/apache/spark/pull/9084#discussion_r41826520
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
    @@ -365,16 +372,13 @@ private[spark] class MemoryStore(blockManager: 
BlockManager, memoryManager: Memo
          * for freeing up more space for another block that needs to be put. 
Only then the actually
          * dropping of blocks (and writing to disk if necessary) can proceed 
in parallel. */
     
    -    accountingLock.synchronized {
    +    memoryManager.synchronized {
           // Note: if we have previously unrolled this block successfully, 
then pending unroll
           // memory should be non-zero. This is the amount that we already 
reserved during the
           // unrolling process. In this case, we can just reuse this space to 
cache our block.
    -      //
    -      // Note: the StaticMemoryManager counts unroll memory as storage 
memory. Here, the
    -      // synchronization on `accountingLock` guarantees that the release 
of unroll memory and
    -      // acquisition of storage memory happens atomically. However, if 
storage memory is acquired
    -      // outside of MemoryStore or if unroll memory is counted as 
execution memory, then we will
    -      // have to revisit this assumption. See SPARK-10983 for more context.
    +      // The synchronization on `memoryManager` here guarantees that the 
release and acquire
    +      // happen atomically. This relies on the assumption that all memory 
acquisitions are
    +      // synchronized on the same lock.
    --- End diff --
    
    It's nice that this simplification resulted from the single-lock approach. 
I'm happy to stick with that simple approach until perf. data suggests that we 
should do something more complex.


---
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