Navigation Menu

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

Only do priming read if there is no space in dsts buffers. #3958

Closed
wants to merge 1 commit into from

Conversation

normanmaurer
Copy link
Member

Motivation:

A SSL_read is needed to ensure the bio buffer is flushed, for this we did a priming read. This can be removed in many cases. Also ensure we always fill as much as possible in the destination buffers.

Modifications:

  • Only do priming read if capacity of all dsts buffers is zero
  • Always produce as must data as possible in the dsts buffers.

Result:

Faster code.

Motivation:

A SSL_read is needed to ensure the bio buffer is flushed, for this we did a priming read. This can be removed in many cases. Also ensure we always fill as much as possible in the destination buffers.

Modifications:

- Only do priming read if capacity of all dsts buffers is zero
- Always produce as must data as possible in the dsts buffers.

Result:

Faster code.
@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 review

int bytesRead = readPlaintextData(dst);

// TODO: We may want to consider if we move this check and only do it in a less often called place at
// the price to be not 100% accurate, like for example when calling SSL.getError(...).
Copy link
Member

Choose a reason for hiding this comment

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

at the price of not being 100% accurate

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@trustin
Copy link
Member

trustin commented Jul 8, 2015

LGTM. Fix the comments though.

@normanmaurer
Copy link
Member Author

Cherry-picked into 4.0 (d345465), 4.1 (8d1c6eb) and master (477010e)

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