xintongsong commented on a change in pull request #9747: [FLINK-13985] Use 
unsafe memory for managed memory
URL: https://github.com/apache/flink/pull/9747#discussion_r335273631
 
 

 ##########
 File path: 
flink-core/src/main/java/org/apache/flink/core/memory/MemoryUtils.java
 ##########
 @@ -57,4 +64,79 @@
 
        /** Should not be instantiated. */
        private MemoryUtils() {}
+
+       private static Constructor<? extends ByteBuffer> 
getDirectBufferPrivateConstructor() {
+               //noinspection OverlyBroadCatchBlock
+               try {
+                       Constructor<? extends ByteBuffer> constructor =
+                               
ByteBuffer.allocateDirect(1).getClass().getDeclaredConstructor(long.class, 
int.class);
+                       constructor.setAccessible(true);
+                       return constructor;
+               } catch (NoSuchMethodException e) {
+                       ExceptionUtils.rethrow(
+                               e,
+                               "The private constructor 
java.nio.DirectByteBuffer.<init>(long, int) is not available.");
+               } catch (SecurityException e) {
+                       ExceptionUtils.rethrow(
+                               e,
+                               "The private constructor 
java.nio.DirectByteBuffer.<init>(long, int) is not available, " +
+                                       "permission denied by security 
manager");
+               } catch (Throwable t) {
+                       ExceptionUtils.rethrow(
+                               t,
+                               "Unclassified error while trying to access 
private constructor " +
+                                       "java.nio.DirectByteBuffer.<init>(long, 
int).");
+               }
+               throw new RuntimeException("unexpected to avoid returning 
null");
+       }
+
+       /**
+        * Allocates unsafe native memory.
+        *
+        * @param size size of the unsafe memory to allocate.
+        * @return address of the allocated unsafe memory
+        */
+       static long allocateUnsafe(long size) {
+               return UNSAFE.allocateMemory(Math.max(1L, size));
+       }
+
+       /**
+        * Creates a cleaner to release the unsafe memory by VM GC.
+        *
+        * <p>When memory owner becomes <a 
href="package-summary.html#reachability">phantom reachable</a>,
+        * GC will release the underlying unsafe memory if not released yet.
+        *
+        * @param owner memory owner which phantom reaching is to monitor by GC 
and release the unsafe memory
+        * @param address address of the unsafe memory to release
+        * @return action to run to release the unsafe memory manually
+        */
+       static Runnable createMemoryGcCleaner(@Nullable Object owner, long 
address) {
+               return createGcCleaner(owner, () -> releaseUnsafe(address));
+       }
+
+       @SuppressWarnings("UseOfSunClasses")
+       private static Runnable createGcCleaner(@Nullable Object owner, 
Runnable toClean) {
+               return owner == null ? toClean : sun.misc.Cleaner.create(owner, 
toClean)::clean;
 
 Review comment:
   Agree with removing null case.
   
   I get the idea that we use `Cleaner#clean` to guarantee that 
`Unsafe#freeMemory` will be called only once. And I think it's a elegant 
design. My point is we should add test cases to protect this. E.g., first call 
`HybridMemorySegment#free` then explicitly trigger a GC, and verify that there 
should be no failures due to multiple calls of `Unsafe#freeMemory`.

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