KarmaGYZ commented on a change in pull request #9693: [FLINK-13984] Separate 
on-heap and off-heap managed memory pools
URL: https://github.com/apache/flink/pull/9693#discussion_r325466726
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/memory/MemoryManager.java
 ##########
 @@ -257,133 +216,98 @@ public boolean verifyEmpty() {
         *                                   of memory pages any more.
         */
        public List<MemorySegment> allocatePages(Object owner, int numPages) 
throws MemoryAllocationException {
-               final ArrayList<MemorySegment> segs = new 
ArrayList<MemorySegment>(numPages);
-               allocatePages(owner, segs, numPages);
-               return segs;
+               List<MemorySegment> segments = new ArrayList<>(numPages);
+               allocatePages(owner, segments, numPages);
+               return segments;
        }
 
        /**
-        * Allocates a set of memory segments from this memory manager. If the 
memory manager pre-allocated the
-        * segments, they will be taken from the pool of memory segments. 
Otherwise, they will be allocated
-        * as part of this call.
+        * Allocates a set of memory segments from this memory manager.
+        *
+        * <p>The returned segments can have any memory type. The total 
allocated memory for each type will not exceed its
+        * size limit, announced in the constructor.
         *
         * @param owner The owner to associate with the memory segment, for the 
fallback release.
         * @param target The list into which to put the allocated memory pages.
         * @param numPages The number of pages to allocate.
         * @throws MemoryAllocationException Thrown, if this memory manager 
does not have the requested amount
         *                                   of memory pages any more.
         */
-       public void allocatePages(Object owner, List<MemorySegment> target, int 
numPages)
-                       throws MemoryAllocationException {
+       public void allocatePages(
+                       Object owner,
+                       Collection<MemorySegment> target,
+                       int numPages) throws MemoryAllocationException {
                // sanity check
-               if (owner == null) {
-                       throw new IllegalArgumentException("The memory owner 
must not be null.");
-               }
+               Preconditions.checkNotNull(owner, "The memory owner must not be 
null.");
+               Preconditions.checkState(!isShutDown, "Memory manager has been 
shut down.");
 
                // reserve array space, if applicable
                if (target instanceof ArrayList) {
                        ((ArrayList<MemorySegment>) 
target).ensureCapacity(numPages);
                }
 
-               // -------------------- BEGIN CRITICAL SECTION 
-------------------
-               synchronized (lock) {
-                       if (isShutDown) {
-                               throw new IllegalStateException("Memory manager 
has been shut down.");
-                       }
-
-                       // in the case of pre-allocated memory, the 
'numNonAllocatedPages' is zero, in the
-                       // lazy case, the 'freeSegments.size()' is zero.
-                       if (numPages > 
(memoryPool.getNumberOfAvailableMemorySegments() + numNonAllocatedPages)) {
-                               throw new MemoryAllocationException("Could not 
allocate " + numPages + " pages. Only " +
-                                               
(memoryPool.getNumberOfAvailableMemorySegments() + numNonAllocatedPages)
-                                               + " pages are remaining.");
-                       }
-
-                       Set<MemorySegment> segmentsForOwner = 
allocatedSegments.get(owner);
-                       if (segmentsForOwner == null) {
-                               segmentsForOwner = new 
HashSet<MemorySegment>(numPages);
-                               allocatedSegments.put(owner, segmentsForOwner);
-                       }
+               // in the case of pre-allocated memory, the 
'numNonAllocatedPages' is zero, in the
 
 Review comment:
   Since we remove memory preallocation, does this comment still valid?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to