From 2dc686ded17d8123118921ac6d36aea30cdce267 Mon Sep 17 00:00:00 2001
From: Tim Brooks <tim@uncontended.net>
Date: Tue, 21 May 2019 23:32:41 -0600
Subject: [PATCH] Prefer direct io buffers if direct buffers pooled (#9167)

Motivation

Direct buffers are normally preferred when interfacing with raw
sockets. Currently netty will only return direct io buffers (for reading
from a channel) when a platform has unsafe. However, this is
inconsistent with the write-side (filterOutboundMessage) where a direct
byte buffer will be returned if pooling is enabled. This means that
environments without unsafe (and no manual netty configurations) end up
with many pooled heap byte buffers for reading, many pooled direct byte
buffers for writing, and jdk pooled byte buffers (for reading).

Modifications

This commit modifies the AbstractByteBufAllocator to return a direct
byte buffer for io handling when the platform has unsafe or direct byte
buffers are pooled.

Result:

Use direct buffers when direct buffers are pooled for IO.
---
 .../buffer/AbstractByteBufAllocator.java      |  6 +++---
 .../buffer/PooledByteBufAllocatorTest.java    | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java
index 920b3fb46d..9e15822b4c 100644
--- a/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java
+++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBufAllocator.java
@@ -127,7 +127,7 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator {
 
     @Override
     public ByteBuf ioBuffer() {
-        if (PlatformDependent.hasUnsafe()) {
+        if (PlatformDependent.hasUnsafe() || isDirectBufferPooled()) {
             return directBuffer(DEFAULT_INITIAL_CAPACITY);
         }
         return heapBuffer(DEFAULT_INITIAL_CAPACITY);
@@ -135,7 +135,7 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator {
 
     @Override
     public ByteBuf ioBuffer(int initialCapacity) {
-        if (PlatformDependent.hasUnsafe()) {
+        if (PlatformDependent.hasUnsafe() || isDirectBufferPooled()) {
             return directBuffer(initialCapacity);
         }
         return heapBuffer(initialCapacity);
@@ -143,7 +143,7 @@ public abstract class AbstractByteBufAllocator implements ByteBufAllocator {
 
     @Override
     public ByteBuf ioBuffer(int initialCapacity, int maxCapacity) {
-        if (PlatformDependent.hasUnsafe()) {
+        if (PlatformDependent.hasUnsafe() || isDirectBufferPooled()) {
             return directBuffer(initialCapacity, maxCapacity);
         }
         return heapBuffer(initialCapacity, maxCapacity);
diff --git a/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java b/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
index b1396373bc..4f9ce334a6 100644
--- a/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
+++ b/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
@@ -92,6 +92,25 @@ public class PooledByteBufAllocatorTest extends AbstractByteBufAllocatorTest<Poo
         heapBuffer.release();
     }
 
+    @Test
+    public void testIOBuffersAreDirectWhenUnsafeAvailableOrDirectBuffersPooled() {
+        PooledByteBufAllocator allocator = newAllocator(true);
+        ByteBuf ioBuffer = allocator.ioBuffer();
+
+        assertTrue(ioBuffer.isDirect());
+        ioBuffer.release();
+
+        PooledByteBufAllocator unpooledAllocator = newUnpooledAllocator();
+        ioBuffer = unpooledAllocator.ioBuffer();
+
+        if (PlatformDependent.hasUnsafe()) {
+            assertTrue(ioBuffer.isDirect());
+        } else {
+            assertFalse(ioBuffer.isDirect());
+        }
+        ioBuffer.release();
+    }
+
     @Test
     public void testWithoutUseCacheForAllThreads() {
         assertFalse(Thread.currentThread() instanceof FastThreadLocalThread);