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

Enhance handshake test to offer a variant of the test in which ssl_ctx objects are allocated per connection #195

Closed
wants to merge 4 commits into from

Conversation

nhorman
Copy link
Contributor

@nhorman nhorman commented May 24, 2024

while best practice is to create a single ssl_ctx object and share it among ssl connections, it often happens that existing code in the field creates a context per connection. to allow us to test performance in this environment, enhance the handshake test with the -s option. Specifying this option alters the test behavior such that contexts are created and freed per connection, rather than the default behavior which is to share a context

We're adding a new option to not share ctx objects, and its getting hard
to manage multiple options, so switch to using getopt
To better understand our performance behavior, add an option to the
handshake test to allow the threads to repeatedly allocate their own
contexts per connection.  If -s is specified this will be the behavior.
Omitting -s uses the default behavior, which is to share an ssl_ctx
among all connections
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Looks good. Can we update the README to mention this new argument?

@mattcaswell
Copy link
Member

A separate thought on this. It would be quite useful to have a performance test which only creates SSL_CTX - since it appears to be this stage which is a significant cause of slow down.

@nhorman
Copy link
Contributor Author

nhorman commented May 29, 2024

@mattcaswell I like the idea of a test for creating ssl_ctx objects. I'll create a separate test for that

@nhorman nhorman requested a review from mattcaswell May 29, 2024 13:31
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved assuming the minor typo is fixed

perf/README Outdated Show resolved Hide resolved
@t8m t8m added the ready label Jun 10, 2024
@t8m
Copy link
Member

t8m commented Jun 10, 2024

Merged. Thank you.

@t8m t8m closed this Jun 10, 2024
openssl-machine pushed a commit that referenced this pull request Jun 10, 2024
We're adding a new option to not share ctx objects, and its getting hard
to manage multiple options, so switch to using getopt

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #195)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2024
To better understand our performance behavior, add an option to the
handshake test to allow the threads to repeatedly allocate their own
contexts per connection.  If -s is specified this will be the behavior.
Omitting -s uses the default behavior, which is to share an ssl_ctx
among all connections

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #195)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2024
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #195)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Augment handshake test to allow reusing of store in ssl_ctx creation
4 participants