[#5770] Use heapbuffers by default when using LocalChannel and LocalS… #5792

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@normanmaurer
Member

…erverChannel.

Motivation:

The local transport is used to communicate in the same JVM so we should use heap buffers.

Modifications:

Use heapbuffers by default if not requested otherwise.

Result:

No allocating of direct buffers by default when using local transport

@normanmaurer normanmaurer [#5770] Use heapbuffers by default when using LocalChannel and LocalS…
…erverChannel.

Motivation:

The local transport is used to communicate in the same JVM so we should use heap buffers.

Modifications:

Use heapbuffers by default if not requested otherwise.

Result:

No allocating of direct buffers by default when using local transport
f7c4e12
@normanmaurer normanmaurer self-assigned this Sep 2, 2016
@normanmaurer normanmaurer added this to the 4.1.6.Final milestone Sep 2, 2016
@nmittler nmittler commented on the diff Sep 2, 2016
...o/netty/channel/local/PreferHeapByteBufAllocator.java
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.channel.local;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.CompositeByteBuf;
+
+/**
+ * Wraps another {@link ByteBufAllocator} and use heapbuffers everywhere except when a direct buffer is explicit
+ * requested.
+ */
+final class PreferHeapByteBufAllocator implements ByteBufAllocator {
@nmittler
nmittler Sep 2, 2016 edited Member

Any thoughts on making this a bit more generic? Something like BufferPreferenceAllocator that can be initialized with a boolean of direct vs heap?

Don't really have any use case yet for anything other than this ... maybe you can think of one?

@normanmaurer
normanmaurer Sep 2, 2016 Member

@nmittler not sure if we should... Do you think it is useful in other cases as well ? If not I would prefer to keep it package-private and we can open up once we need too. WDYT ?

@nmittler
nmittler Sep 2, 2016 Member

meh ... it's fine as-is. Just a thought :)

@nmittler
Member
nmittler commented Sep 2, 2016
@Scottmitch

lgtm

@normanmaurer
Member

Cherry-picked into 4.1 (6c8cd02)

@normanmaurer normanmaurer deleted the heap_for_local branch Oct 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment