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

    https://github.com/apache/spark/pull/16499#discussion_r104770778
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
    @@ -1048,7 +1065,7 @@ private[spark] class BlockManager(
               try {
                 replicate(blockId, bytesToReplicate, level, remoteClassTag)
               } finally {
    -            bytesToReplicate.dispose()
    +            bytesToReplicate.unmap()
    --- End diff --
    
    I would to explore this further to ensure this would really work well, but 
I like your idea with one caveat. I think we should avoid using the `disposed` 
var for this purpose. Using it this way would introduce ambiguity in its 
meaning when `disposed` is true. In addition to its current meaning, it could 
also mean "this buffer is not disposed but we don't want to dispose it".
    
    Instead, I suggest keeping the usage of `disposed` as-is and adding an 
additional var, e.g. `indisposable`, which makes the `dispose` method itself a 
no-op.


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