Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

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

Add support for recvmmsg(...) even with connected datagram channels w… #9539

Merged
merged 3 commits into from Sep 6, 2019

Conversation

@normanmaurer
Copy link
Member

normanmaurer commented Sep 4, 2019

…hen using native epoll transport

Motivation:

394a1b3 added support for recvmmsg(...) for unconnected datagram channels, this change also allows to use recvmmsg(...) with connected datagram channels.

Modifications:

  • Always try to use recvmmsg(...) if configured to do so
  • Adjust unit test to cover it

Result:

Less syscalls when reading datagram packets

…hen using native epoll transport

Motivation:

394a1b3 added support for recvmmsg(...) for unconnected datagram channels, this change also allows to use recvmmsg(...) with connected datagram channels.

Modifications:

- Always try to use recvmmsg(...) if configured to do so
- Adjust unit test to cover it

Result:

Less syscalls when reading datagram packets
@normanmaurer normanmaurer added this to the 4.1.40.Final milestone Sep 4, 2019
@normanmaurer normanmaurer requested review from franz1981 and njhill Sep 4, 2019
: byteBuf.writableBytes();
allocHandle.attemptedBytesRead(writable);

byteBuf.writerIndex(byteBuf.readerIndex() + writable);

This comment has been minimized.

Copy link
@njhill

njhill Sep 6, 2019

Member

I'm not sure this is right.. isn't it the capacity that would need to be reduced in the case that 0 < maxDatagramPacketSize < byteBuf.writeableBytes()? Though i'm not sure we would really want to do that since it could involve actually reallocation/copying internally...

This comment has been minimized.

Copy link
@normanmaurer

normanmaurer Sep 6, 2019

Author Member

@njhil arg... I think you are right and it should be: byteBuf.writerIndex(byteBuf.capacity() - writable);

This comment has been minimized.

Copy link
@normanmaurer

normanmaurer Sep 6, 2019

Author Member

also I think we should update the readerIndex to match this.

This comment has been minimized.

Copy link
@normanmaurer

normanmaurer Sep 6, 2019

Author Member

actually also this is not 100 % correct... Should be good now with the latest change. PTAL again

@normanmaurer normanmaurer requested a review from njhill Sep 6, 2019
@njhill
njhill approved these changes Sep 6, 2019
@normanmaurer normanmaurer merged commit 6bc2da6 into 4.1 Sep 6, 2019
3 checks passed
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 normanmaurer deleted the scattering_read_connected_datagram branch Sep 6, 2019
normanmaurer added a commit that referenced this pull request Sep 6, 2019
#9539)


Motivation:

394a1b3 added support for recvmmsg(...) for unconnected datagram channels, this change also allows to use recvmmsg(...) with connected datagram channels.

Modifications:

- Always try to use recvmmsg(...) if configured to do so
- Adjust unit test to cover it

Result:

Less syscalls when reading datagram packets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.