[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7621: GEODE-10036: remove need to export sun.nio.ch

[GitBox <gi...@apache.org>]

pivotal-jbarrett commented on code in PR #7621:
URL: https://github.com/apache/geode/pull/7621#discussion_r859077733


##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -90,46 +87,46 @@ static boolean computeUseDirectBuffers() {
    *
    * @return a byte buffer to be used for sending on this connection.
    */
-  public ByteBuffer acquireDirectSenderBuffer(int size) {
+  public PooledByteBuffer acquireDirectSenderBuffer(int size) {
     return acquireDirectBuffer(size, true);
   }
 
-  public ByteBuffer acquireDirectReceiveBuffer(int size) {
+  public PooledByteBuffer acquireDirectReceiveBuffer(int size) {
     return acquireDirectBuffer(size, false);
   }
 
   /**
    * try to acquire direct buffer, if enabled by configuration
    */
-  private ByteBuffer acquireDirectBuffer(int size, boolean send) {
-    ByteBuffer result;
+  private PooledByteBuffer acquireDirectBuffer(int size, boolean send) {
+    ByteBuffer byteBuffer;
 
     if (useDirectBuffers) {
       if (size <= MEDIUM_BUFFER_SIZE) {
-        result = acquirePredefinedFixedBuffer(send, size);
+        byteBuffer = acquirePredefinedFixedBuffer(send, size);
       } else {
-        result = acquireLargeBuffer(send, size);
+        byteBuffer = acquireLargeBuffer(send, size);
       }
-      if (result.capacity() > size) {
-        result.position(0).limit(size);
-        result = result.slice();
+      if (byteBuffer.capacity() > size) {
+        byteBuffer.position(0).limit(size);
+        return new PooledByteBuffer(byteBuffer, byteBuffer.slice());

Review Comment:
   Going from 1 to 2 transient allocations here.



##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -90,46 +87,46 @@ static boolean computeUseDirectBuffers() {
    *
    * @return a byte buffer to be used for sending on this connection.
    */
-  public ByteBuffer acquireDirectSenderBuffer(int size) {
+  public PooledByteBuffer acquireDirectSenderBuffer(int size) {
     return acquireDirectBuffer(size, true);
   }
 
-  public ByteBuffer acquireDirectReceiveBuffer(int size) {
+  public PooledByteBuffer acquireDirectReceiveBuffer(int size) {
     return acquireDirectBuffer(size, false);
   }
 
   /**
    * try to acquire direct buffer, if enabled by configuration
    */
-  private ByteBuffer acquireDirectBuffer(int size, boolean send) {
-    ByteBuffer result;
+  private PooledByteBuffer acquireDirectBuffer(int size, boolean send) {
+    ByteBuffer byteBuffer;
 
     if (useDirectBuffers) {
       if (size <= MEDIUM_BUFFER_SIZE) {
-        result = acquirePredefinedFixedBuffer(send, size);
+        byteBuffer = acquirePredefinedFixedBuffer(send, size);
       } else {
-        result = acquireLargeBuffer(send, size);
+        byteBuffer = acquireLargeBuffer(send, size);
       }
-      if (result.capacity() > size) {
-        result.position(0).limit(size);
-        result = result.slice();
+      if (byteBuffer.capacity() > size) {
+        byteBuffer.position(0).limit(size);
+        return new PooledByteBuffer(byteBuffer, byteBuffer.slice());
       }
-      return result;
+      return new PooledByteBuffer(byteBuffer);
     }
     // if we are using heap buffers then don't bother with keeping them around
-    result = ByteBuffer.allocate(size);
+    byteBuffer = ByteBuffer.allocate(size);
     updateBufferStats(size, send, false);
-    return result;
+    return new PooledByteBuffer(byteBuffer);

Review Comment:
   We don't pool heap buffers, so why the `PooledByteBuffer` wrapper here?



##########
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java:
##########
@@ -90,46 +87,46 @@ static boolean computeUseDirectBuffers() {
    *
    * @return a byte buffer to be used for sending on this connection.
    */
-  public ByteBuffer acquireDirectSenderBuffer(int size) {
+  public PooledByteBuffer acquireDirectSenderBuffer(int size) {
     return acquireDirectBuffer(size, true);
   }
 
-  public ByteBuffer acquireDirectReceiveBuffer(int size) {
+  public PooledByteBuffer acquireDirectReceiveBuffer(int size) {
     return acquireDirectBuffer(size, false);
   }
 
   /**
    * try to acquire direct buffer, if enabled by configuration
    */
-  private ByteBuffer acquireDirectBuffer(int size, boolean send) {
-    ByteBuffer result;
+  private PooledByteBuffer acquireDirectBuffer(int size, boolean send) {
+    ByteBuffer byteBuffer;
 
     if (useDirectBuffers) {
       if (size <= MEDIUM_BUFFER_SIZE) {
-        result = acquirePredefinedFixedBuffer(send, size);
+        byteBuffer = acquirePredefinedFixedBuffer(send, size);
       } else {
-        result = acquireLargeBuffer(send, size);
+        byteBuffer = acquireLargeBuffer(send, size);
       }
-      if (result.capacity() > size) {
-        result.position(0).limit(size);
-        result = result.slice();
+      if (byteBuffer.capacity() > size) {
+        byteBuffer.position(0).limit(size);
+        return new PooledByteBuffer(byteBuffer, byteBuffer.slice());
       }
-      return result;
+      return new PooledByteBuffer(byteBuffer);

Review Comment:
   Going from 0 to 1 transient allocations here.



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

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org