Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11874#discussion_r57253450
--- Diff:
core/src/main/scala/org/apache/spark/memory/StaticMemoryManager.scala ---
@@ -62,17 +64,34 @@ private[spark] class StaticMemoryManager(
}
}
- override def acquireUnrollMemory(blockId: BlockId, numBytes: Long):
Boolean = synchronized {
- val currentUnrollMemory =
storageMemoryPool.memoryStore.currentUnrollMemory
- val freeMemory = storageMemoryPool.memoryFree
+ override def acquireUnrollMemory(blockId: BlockId, numBytes: Long):
Boolean = {
// When unrolling, we will use all of the existing free memory, and,
if necessary,
// some extra space freed from evicting cached blocks. We must place a
cap on the
// amount of memory to be evicted by unrolling, however, otherwise
unrolling one
// big block can blow away the entire cache.
- val maxNumBytesToFree = math.max(0, maxUnrollMemory -
currentUnrollMemory - freeMemory)
- // Keep it within the range 0 <= X <= maxNumBytesToFree
- val numBytesToFree = math.max(0, math.min(maxNumBytesToFree, numBytes
- freeMemory))
- storageMemoryPool.acquireMemory(blockId, numBytes, numBytesToFree)
+ val (freeMemoryAcquired, numBytesToFree) = synchronized {
+ val currentUnrollMemory =
storageMemoryPool.memoryStore.currentUnrollMemory
+ val freeMemory = storageMemoryPool.memoryFree
+ val freeMemoryAcquired = math.min(numBytes, freeMemory)
+ val res = storageMemoryPool.acquireMemory(blockId,
freeMemoryAcquired, numBytesToFree = 0)
+ assert(res, "failed to acquire free storage memory")
+ val maxNumBytesToFree = math.max(0, maxUnrollMemory -
currentUnrollMemory - freeMemory)
+ // Keep it within the range 0 <= X <= maxNumBytesToFree
+ val numBytesToFree = math.min(numBytes - freeMemoryAcquired,
maxNumBytesToFree)
+ (freeMemoryAcquired, numBytesToFree)
+ }
+ try {
+ if (storageMemoryPool.acquireMemory(blockId, numBytes -
freeMemoryAcquired, numBytesToFree)) {
--- End diff --
actually, don't you want to check whether `numBytes - freeMemoryAcquired`
is > 0? If it's 0 then we should probably just return true here without calling
anything.
---
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]
