-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-10984] Simplify *MemoryManager class structure #9127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b7c9c23
25ba4b5
3d997ce
d9e6b84
98ef86b
8f93e94
3bbc54d
88a7970
ec48ff9
6f98bc4
6459397
60c66b2
7d6a37f
0dc21dc
f21b767
46ad693
c33e330
ef45d91
c7eac69
d86f435
bba5550
66ae259
b1d5151
d0c0dd9
48149fc
c8ba196
63a6cbc
6ec9c30
1593fad
64bec0b
f9240e9
a95bc08
b3ad761
a7e8320
e874a45
2ba6e51
0c13723
04ec429
e56d039
aa14113
7addf8b
5af0b17
a264703
f2ab708
0b5c72f
f68fdb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,14 +15,16 @@ | |
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.unsafe.memory; | ||
| package org.apache.spark.memory; | ||
|
|
||
| import java.util.*; | ||
|
|
||
| import com.google.common.annotations.VisibleForTesting; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import org.apache.spark.unsafe.memory.MemoryBlock; | ||
|
|
||
| /** | ||
| * Manages the memory allocated by an individual task. | ||
| * <p> | ||
|
|
@@ -87,13 +89,9 @@ public class TaskMemoryManager { | |
| */ | ||
| private final BitSet allocatedPages = new BitSet(PAGE_TABLE_SIZE); | ||
|
|
||
| /** | ||
| * Tracks memory allocated with {@link TaskMemoryManager#allocate(long)}, used to detect / clean | ||
| * up leaked memory. | ||
| */ | ||
| private final HashSet<MemoryBlock> allocatedNonPageMemory = new HashSet<MemoryBlock>(); | ||
| private final MemoryManager memoryManager; | ||
|
|
||
| private final ExecutorMemoryManager executorMemoryManager; | ||
| private final long taskAttemptId; | ||
|
|
||
| /** | ||
| * Tracks whether we're in-heap or off-heap. For off-heap, we short-circuit most of these methods | ||
|
|
@@ -103,16 +101,38 @@ public class TaskMemoryManager { | |
| private final boolean inHeap; | ||
|
|
||
| /** | ||
| * Construct a new MemoryManager. | ||
| * Construct a new TaskMemoryManager. | ||
| */ | ||
| public TaskMemoryManager(ExecutorMemoryManager executorMemoryManager) { | ||
| this.inHeap = executorMemoryManager.inHeap; | ||
| this.executorMemoryManager = executorMemoryManager; | ||
| public TaskMemoryManager(MemoryManager memoryManager, long taskAttemptId) { | ||
| this.inHeap = memoryManager.tungstenMemoryIsAllocatedInHeap(); | ||
| this.memoryManager = memoryManager; | ||
| this.taskAttemptId = taskAttemptId; | ||
| } | ||
|
|
||
| /** | ||
| * Acquire N bytes of memory for execution, evicting cached blocks if necessary. | ||
| * @return number of bytes successfully granted (<= N). | ||
| */ | ||
| public long acquireExecutionMemory(long size) { | ||
| return memoryManager.acquireExecutionMemory(size, taskAttemptId); | ||
| } | ||
|
|
||
| /** | ||
| * Release N bytes of execution memory. | ||
| */ | ||
| public void releaseExecutionMemory(long size) { | ||
| memoryManager.releaseExecutionMemory(size, taskAttemptId); | ||
| } | ||
|
|
||
| public long pageSizeBytes() { | ||
| return memoryManager.pageSizeBytes(); | ||
| } | ||
|
|
||
| /** | ||
| * Allocate a block of memory that will be tracked in the MemoryManager's page table; this is | ||
| * intended for allocating large blocks of memory that will be shared between operators. | ||
| * intended for allocating large blocks of Tungsten memory that will be shared between operators. | ||
| * | ||
| * Returns `null` if there was not enough memory to allocate the page. | ||
| */ | ||
| public MemoryBlock allocatePage(long size) { | ||
| if (size > MAXIMUM_PAGE_SIZE_BYTES) { | ||
|
|
@@ -129,7 +149,15 @@ public MemoryBlock allocatePage(long size) { | |
| } | ||
| allocatedPages.set(pageNumber); | ||
| } | ||
| final MemoryBlock page = executorMemoryManager.allocate(size); | ||
| final long acquiredExecutionMemory = acquireExecutionMemory(size); | ||
| if (acquiredExecutionMemory != size) { | ||
| releaseExecutionMemory(acquiredExecutionMemory); | ||
| synchronized (this) { | ||
| allocatedPages.clear(pageNumber); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to release the memory we acquired here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, good catch. Yes. |
||
| } | ||
| return null; | ||
| } | ||
| final MemoryBlock page = memoryManager.tungstenMemoryAllocator().allocate(size); | ||
| page.pageNumber = pageNumber; | ||
| pageTable[pageNumber] = page; | ||
| if (logger.isTraceEnabled()) { | ||
|
|
@@ -152,45 +180,16 @@ public void freePage(MemoryBlock page) { | |
| if (logger.isTraceEnabled()) { | ||
| logger.trace("Freed page number {} ({} bytes)", page.pageNumber, page.size()); | ||
| } | ||
| // Cannot access a page once it's freed. | ||
| executorMemoryManager.free(page); | ||
| } | ||
|
|
||
| /** | ||
| * Allocates a contiguous block of memory. Note that the allocated memory is not guaranteed | ||
| * to be zeroed out (call `zero()` on the result if this is necessary). This method is intended | ||
| * to be used for allocating operators' internal data structures. For data pages that you want to | ||
| * exchange between operators, consider using {@link TaskMemoryManager#allocatePage(long)}, since | ||
| * that will enable intra-memory pointers (see | ||
| * {@link TaskMemoryManager#encodePageNumberAndOffset(MemoryBlock, long)} and this class's | ||
| * top-level Javadoc for more details). | ||
| */ | ||
| public MemoryBlock allocate(long size) throws OutOfMemoryError { | ||
| assert(size > 0) : "Size must be positive, but got " + size; | ||
| final MemoryBlock memory = executorMemoryManager.allocate(size); | ||
| synchronized(allocatedNonPageMemory) { | ||
| allocatedNonPageMemory.add(memory); | ||
| } | ||
| return memory; | ||
| } | ||
|
|
||
| /** | ||
| * Free memory allocated by {@link TaskMemoryManager#allocate(long)}. | ||
| */ | ||
| public void free(MemoryBlock memory) { | ||
| assert (memory.pageNumber == -1) : "Should call freePage() for pages, not free()"; | ||
| executorMemoryManager.free(memory); | ||
| synchronized(allocatedNonPageMemory) { | ||
| final boolean wasAlreadyRemoved = !allocatedNonPageMemory.remove(memory); | ||
| assert (!wasAlreadyRemoved) : "Called free() on memory that was already freed!"; | ||
| } | ||
| long pageSize = page.size(); | ||
| memoryManager.tungstenMemoryAllocator().free(page); | ||
| releaseExecutionMemory(pageSize); | ||
| } | ||
|
|
||
| /** | ||
| * Given a memory page and offset within that page, encode this address into a 64-bit long. | ||
| * This address will remain valid as long as the corresponding page has not been freed. | ||
| * | ||
| * @param page a data page allocated by {@link TaskMemoryManager#allocate(long)}. | ||
| * @param page a data page allocated by {@link TaskMemoryManager#allocatePage(long)}/ | ||
| * @param offsetInPage an offset in this page which incorporates the base offset. In other words, | ||
| * this should be the value that you would pass as the base offset into an | ||
| * UNSAFE call (e.g. page.baseOffset() + something). | ||
|
|
@@ -270,17 +269,15 @@ public long cleanUpAllAllocatedMemory() { | |
| } | ||
| } | ||
|
|
||
| synchronized (allocatedNonPageMemory) { | ||
| final Iterator<MemoryBlock> iter = allocatedNonPageMemory.iterator(); | ||
| while (iter.hasNext()) { | ||
| final MemoryBlock memory = iter.next(); | ||
| freedBytes += memory.size(); | ||
| // We don't call free() here because that calls Set.remove, which would lead to a | ||
| // ConcurrentModificationException here. | ||
| executorMemoryManager.free(memory); | ||
| iter.remove(); | ||
| } | ||
| } | ||
| freedBytes += memoryManager.releaseAllExecutionMemoryForTask(taskAttemptId); | ||
|
|
||
| return freedBytes; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the memory consumption, in bytes, for the current task | ||
| */ | ||
| public long getMemoryConsumptionForThisTask() { | ||
| return memoryManager.getExecutionMemoryUsageForTask(taskAttemptId); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,14 +33,13 @@ | |
| import org.apache.spark.executor.ShuffleWriteMetrics; | ||
| import org.apache.spark.serializer.DummySerializerInstance; | ||
| import org.apache.spark.serializer.SerializerInstance; | ||
| import org.apache.spark.shuffle.ShuffleMemoryManager; | ||
| import org.apache.spark.storage.BlockManager; | ||
| import org.apache.spark.storage.DiskBlockObjectWriter; | ||
| import org.apache.spark.storage.TempShuffleBlockId; | ||
| import org.apache.spark.unsafe.Platform; | ||
| import org.apache.spark.unsafe.array.ByteArrayMethods; | ||
| import org.apache.spark.unsafe.memory.MemoryBlock; | ||
| import org.apache.spark.unsafe.memory.TaskMemoryManager; | ||
| import org.apache.spark.memory.TaskMemoryManager; | ||
| import org.apache.spark.util.Utils; | ||
|
|
||
| /** | ||
|
|
@@ -72,7 +71,6 @@ final class ShuffleExternalSorter { | |
| @VisibleForTesting | ||
| final int maxRecordSizeBytes; | ||
| private final TaskMemoryManager taskMemoryManager; | ||
| private final ShuffleMemoryManager shuffleMemoryManager; | ||
| private final BlockManager blockManager; | ||
| private final TaskContext taskContext; | ||
| private final ShuffleWriteMetrics writeMetrics; | ||
|
|
@@ -105,15 +103,13 @@ final class ShuffleExternalSorter { | |
|
|
||
| public ShuffleExternalSorter( | ||
| TaskMemoryManager memoryManager, | ||
| ShuffleMemoryManager shuffleMemoryManager, | ||
| BlockManager blockManager, | ||
| TaskContext taskContext, | ||
| int initialSize, | ||
| int numPartitions, | ||
| SparkConf conf, | ||
| ShuffleWriteMetrics writeMetrics) throws IOException { | ||
| this.taskMemoryManager = memoryManager; | ||
| this.shuffleMemoryManager = shuffleMemoryManager; | ||
| this.blockManager = blockManager; | ||
| this.taskContext = taskContext; | ||
| this.initialSize = initialSize; | ||
|
|
@@ -124,7 +120,7 @@ public ShuffleExternalSorter( | |
| this.numElementsForSpillThreshold = | ||
| conf.getLong("spark.shuffle.spill.numElementsForceSpillThreshold", Long.MAX_VALUE); | ||
| this.pageSizeBytes = (int) Math.min( | ||
| PackedRecordPointer.MAXIMUM_PAGE_SIZE_BYTES, shuffleMemoryManager.pageSizeBytes()); | ||
| PackedRecordPointer.MAXIMUM_PAGE_SIZE_BYTES, taskMemoryManager.pageSizeBytes()); | ||
| this.maxRecordSizeBytes = pageSizeBytes - 4; | ||
| this.writeMetrics = writeMetrics; | ||
| initializeForWriting(); | ||
|
|
@@ -140,9 +136,9 @@ public ShuffleExternalSorter( | |
| private void initializeForWriting() throws IOException { | ||
| // TODO: move this sizing calculation logic into a static method of sorter: | ||
| final long memoryRequested = initialSize * 8L; | ||
| final long memoryAcquired = shuffleMemoryManager.tryToAcquire(memoryRequested); | ||
| final long memoryAcquired = taskMemoryManager.acquireExecutionMemory(memoryRequested); | ||
| if (memoryAcquired != memoryRequested) { | ||
| shuffleMemoryManager.release(memoryAcquired); | ||
| taskMemoryManager.releaseExecutionMemory(memoryAcquired); | ||
| throw new IOException("Could not acquire " + memoryRequested + " bytes of memory"); | ||
| } | ||
|
|
||
|
|
@@ -272,6 +268,7 @@ private void writeSortedFile(boolean isLastFile) throws IOException { | |
| */ | ||
| @VisibleForTesting | ||
| void spill() throws IOException { | ||
| assert(inMemSorter != null); | ||
| logger.info("Thread {} spilling sort data of {} to disk ({} {} so far)", | ||
| Thread.currentThread().getId(), | ||
| Utils.bytesToString(getMemoryUsage()), | ||
|
|
@@ -281,7 +278,7 @@ void spill() throws IOException { | |
| writeSortedFile(false); | ||
| final long inMemSorterMemoryUsage = inMemSorter.getMemoryUsage(); | ||
| inMemSorter = null; | ||
| shuffleMemoryManager.release(inMemSorterMemoryUsage); | ||
| taskMemoryManager.releaseExecutionMemory(inMemSorterMemoryUsage); | ||
| final long spillSize = freeMemory(); | ||
| taskContext.taskMetrics().incMemoryBytesSpilled(spillSize); | ||
|
|
||
|
|
@@ -316,9 +313,13 @@ private long freeMemory() { | |
| long memoryFreed = 0; | ||
| for (MemoryBlock block : allocatedPages) { | ||
| taskMemoryManager.freePage(block); | ||
| shuffleMemoryManager.release(block.size()); | ||
| memoryFreed += block.size(); | ||
| } | ||
| if (inMemSorter != null) { | ||
| long sorterMemoryUsage = inMemSorter.getMemoryUsage(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when do we release this now?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This memory is now released as part of |
||
| inMemSorter = null; | ||
| taskMemoryManager.releaseExecutionMemory(sorterMemoryUsage); | ||
| } | ||
| allocatedPages.clear(); | ||
| currentPage = null; | ||
| currentPagePosition = -1; | ||
|
|
@@ -337,8 +338,9 @@ public void cleanupResources() { | |
| } | ||
| } | ||
| if (inMemSorter != null) { | ||
| shuffleMemoryManager.release(inMemSorter.getMemoryUsage()); | ||
| long sorterMemoryUsage = inMemSorter.getMemoryUsage(); | ||
| inMemSorter = null; | ||
| taskMemoryManager.releaseExecutionMemory(sorterMemoryUsage); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -353,21 +355,20 @@ private void growPointerArrayIfNecessary() throws IOException { | |
| logger.debug("Attempting to expand sort pointer array"); | ||
| final long oldPointerArrayMemoryUsage = inMemSorter.getMemoryUsage(); | ||
| final long memoryToGrowPointerArray = oldPointerArrayMemoryUsage * 2; | ||
| final long memoryAcquired = shuffleMemoryManager.tryToAcquire(memoryToGrowPointerArray); | ||
| final long memoryAcquired = taskMemoryManager.acquireExecutionMemory(memoryToGrowPointerArray); | ||
| if (memoryAcquired < memoryToGrowPointerArray) { | ||
| shuffleMemoryManager.release(memoryAcquired); | ||
| taskMemoryManager.releaseExecutionMemory(memoryAcquired); | ||
| spill(); | ||
| } else { | ||
| inMemSorter.expandPointerArray(); | ||
| shuffleMemoryManager.release(oldPointerArrayMemoryUsage); | ||
| taskMemoryManager.releaseExecutionMemory(oldPointerArrayMemoryUsage); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Allocates more memory in order to insert an additional record. This will request additional | ||
| * memory from the {@link ShuffleMemoryManager} and spill if the requested memory can not be | ||
| * obtained. | ||
| * memory from the memory manager and spill if the requested memory can not be obtained. | ||
| * | ||
| * @param requiredSpace the required space in the data page, in bytes, including space for storing | ||
| * the record size. This must be less than or equal to the page size (records | ||
|
|
@@ -386,17 +387,14 @@ private void acquireNewPageIfNecessary(int requiredSpace) throws IOException { | |
| throw new IOException("Required space " + requiredSpace + " is greater than page size (" + | ||
| pageSizeBytes + ")"); | ||
| } else { | ||
| final long memoryAcquired = shuffleMemoryManager.tryToAcquire(pageSizeBytes); | ||
| if (memoryAcquired < pageSizeBytes) { | ||
| shuffleMemoryManager.release(memoryAcquired); | ||
| currentPage = taskMemoryManager.allocatePage(pageSizeBytes); | ||
| if (currentPage == null) { | ||
| spill(); | ||
| final long memoryAcquiredAfterSpilling = shuffleMemoryManager.tryToAcquire(pageSizeBytes); | ||
| if (memoryAcquiredAfterSpilling != pageSizeBytes) { | ||
| shuffleMemoryManager.release(memoryAcquiredAfterSpilling); | ||
| currentPage = taskMemoryManager.allocatePage(pageSizeBytes); | ||
| if (currentPage == null) { | ||
| throw new IOException("Unable to acquire " + pageSizeBytes + " bytes of memory"); | ||
| } | ||
| } | ||
| currentPage = taskMemoryManager.allocatePage(pageSizeBytes); | ||
| currentPagePosition = currentPage.getBaseOffset(); | ||
| freeSpaceInCurrentPage = pageSizeBytes; | ||
| allocatedPages.add(currentPage); | ||
|
|
@@ -430,17 +428,14 @@ public void insertRecord( | |
| long overflowPageSize = ByteArrayMethods.roundNumberOfBytesToNearestWord(totalSpaceRequired); | ||
| // The record is larger than the page size, so allocate a special overflow page just to hold | ||
| // that record. | ||
| final long memoryGranted = shuffleMemoryManager.tryToAcquire(overflowPageSize); | ||
| if (memoryGranted != overflowPageSize) { | ||
| shuffleMemoryManager.release(memoryGranted); | ||
| MemoryBlock overflowPage = taskMemoryManager.allocatePage(overflowPageSize); | ||
| if (overflowPage == null) { | ||
| spill(); | ||
| final long memoryGrantedAfterSpill = shuffleMemoryManager.tryToAcquire(overflowPageSize); | ||
| if (memoryGrantedAfterSpill != overflowPageSize) { | ||
| shuffleMemoryManager.release(memoryGrantedAfterSpill); | ||
| overflowPage = taskMemoryManager.allocatePage(overflowPageSize); | ||
| if (overflowPage == null) { | ||
| throw new IOException("Unable to acquire " + overflowPageSize + " bytes of memory"); | ||
| } | ||
| } | ||
| MemoryBlock overflowPage = taskMemoryManager.allocatePage(overflowPageSize); | ||
| allocatedPages.add(overflowPage); | ||
| dataPage = overflowPage; | ||
| dataPagePosition = overflowPage.getBaseOffset(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
@return