Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer direct io buffers if direct buffers pooled #9167

Merged
merged 1 commit into from May 22, 2019

Conversation

Projects
None yet
7 participants
@tbrooks8
Copy link
Contributor

commented May 21, 2019

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.

@tbrooks8

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

As I understand it, Netty prefers heap buffers when unsafe is disabled due to allocation costs. However, it seems like if pooling is enabled this cost will be reduced, and this is the approach Netty takes on the write-side?

@netty-bot

This comment has been minimized.

Copy link

commented May 21, 2019

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@tbrooks8 good catch! Thats a good change... thanks a lot

@normanmaurer normanmaurer requested review from ejona86 and njhill May 21, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@netty-bot test this please

@ejona86
Copy link
Member

left a comment

I just noticed this last week and thought it was strange.

@franz1981
Copy link
Contributor

left a comment

LGTM good catch!!!

@normanmaurer normanmaurer added this to the 4.1.37.Final milestone May 21, 2019

@tbrooks8

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

@normanmaurer - you're welcome. It's something I've seen for a while in Elasticsearch, so I'm glad I finally got around to looking into it. Hopefully it will reduce memory usage for anyone that runs Netty without unsafe but with direct buffers.

Prefer direct io buffers if direct buffers pooled
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.

@tbrooks8 tbrooks8 force-pushed the tbrooks8:use_direct_io_for_reads branch from 4cd7705 to eec090d May 21, 2019

@tbrooks8

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

There was an extra whitespace line that I think was failing check style. I removed it, squashed, and force pushed.

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@netty-bot test this please

@njhill

njhill approved these changes May 21, 2019

Copy link
Member

left a comment

Nice change, thanks @tbrooks8!

@johnou

johnou approved these changes May 21, 2019

@normanmaurer normanmaurer merged commit 2dc686d into netty:4.1 May 22, 2019

3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details

normanmaurer added a commit that referenced this pull request May 22, 2019

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.