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

Allow efficient writing of CompositeByteBuf when using native epoll tran... #2789

Closed
wants to merge 1 commit into from

Conversation

normanmaurer
Copy link
Member

...sport.

Motivation:

There were no way to efficient write a CompositeByteBuf as we always did a memory copy to a direct buffer in this case. This is not needed as we can just write a CompositeByteBuf as long as all the components are buffers with a memory address.

Modifications:

  • Introduce CompositeByteBuf.internalReplaceComponent(...) which allows to replace a ByteBuf in a CompositeByteBuf in an efficient way.
  • Make use of internalReplaceComponent(...) to replace all ByteBuf in the CompositeByteBuf that have no memory address.
  • Also handle CompositeByteBuf that have more components then 1024.

Result:

More efficient writing of CompositeByteBuf.

@normanmaurer
Copy link
Member Author

@trustin please review

@ghost
Copy link

ghost commented Aug 19, 2014

Build result for #2789 at db45db4: Failure

@ghost
Copy link

ghost commented Aug 19, 2014

Build result for #2789 at bf52fd2: Failure

@ghost
Copy link

ghost commented Aug 19, 2014

Build result for #2789 at ab33816: Success

@normanmaurer
Copy link
Member Author

@trustin ok now it is correct. please review

@ghost
Copy link

ghost commented Aug 19, 2014

Build result for #2789 at 5965394: Success

// passed to write.
buf = newDirectBuffer(buf);
assert buf.hasMemoryAddress();
boolean isDirect = buf.isDirect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be evaluated when buf.hasMemoryAddress() returns true or PlatformDependent.hasUnsafe() returns true. We should not evaluate it at least when buf.hasMemoryAddress() returns true.

@trustin
Copy link
Member

trustin commented Aug 21, 2014

Looks good. Please merge once my comment is addressed.

…ransport.

Motivation:

There were no way to efficient write a CompositeByteBuf as we always did a memory copy to a direct buffer in this case. This is not needed as we can just write a CompositeByteBuf as long as all the components are buffers with a memory address.

Modifications:

- Write CompositeByteBuf which contains only direct buffers without memory copy
- Also handle CompositeByteBuf that have more components then 1024.

Result:

More efficient writing of CompositeByteBuf.
@ghost
Copy link

ghost commented Aug 21, 2014

Build result for #2789 at 6f09efc: Success

@normanmaurer normanmaurer deleted the epoll_composite_write branch August 21, 2014 08:59
@normanmaurer
Copy link
Member Author

@trustin cherry-picked into 4.0, 4.1 and master after addressing your comment

@normanmaurer normanmaurer added this to the 4.0.24.Final milestone Aug 21, 2014
@normanmaurer normanmaurer self-assigned this Aug 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants