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

Better handling of BUFFER_OVERFLOW when unwrap data. #3959

Closed
wants to merge 1 commit into from

Conversation

normanmaurer
Copy link
Member

Motivation:

When we detect a BUFFER_OVERFLOW we should just forward the already produced data and allocate a new buffer and NOT do any extra memory copies while trying to expand the buffer.

Modifications:

When a BUFFER_OVERFLOW is returned and some data was produced just fire this data through the pipeline and allocate a new buffer to read again.

Result:

Less memorycopies and so better performance.

@normanmaurer normanmaurer self-assigned this Jul 8, 2015
@normanmaurer normanmaurer added this to the 4.0.30.Final milestone Jul 8, 2015
@normanmaurer
Copy link
Member Author

@trustin please check

in.nioBuffer(readerIndex, len);

ByteBuffer out0 = out.nioBufferCount() == 1 ? out.internalNioBuffer(writerIndex, out.writableBytes()) :
out.nioBuffer(writerIndex, out.writableBytes());
Copy link
Member

Choose a reason for hiding this comment

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

You could extract this into a static method, because you do this 3 times in this change.

@normanmaurer
Copy link
Member Author

@trustin only this comment ?

Motivation:

When we detect a BUFFER_OVERFLOW we should just forward the already produced data and allocate a new buffer and NOT do any extra memory copies while trying to expand the buffer.

Modifications:

When a BUFFER_OVERFLOW is returned and some data was produced just fire this data through the pipeline and allocate a new buffer to read again.

Result:

Less memorycopies and so better performance.
@trustin
Copy link
Member

trustin commented Jul 8, 2015

@normanmaurer yep. LGTM now.

@normanmaurer
Copy link
Member Author

Cherry-picked into 4.0 (513f405) , 4.1 (9660e2f) and master (93b7625)

@normanmaurer normanmaurer deleted the sslhandler_buffer_overflow branch July 8, 2015 08:36
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