[
https://issues.apache.org/jira/browse/PROTON-2137?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16980274#comment-16980274
]
ASF GitHub Bot commented on PROTON-2137:
----------------------------------------
astitcher commented on pull request #210: PROTON-2137: Removing ssl init from
ssl_server_options default constructor
URL: https://github.com/apache/qpid-proton/pull/210#discussion_r349672089
##########
File path: cpp/src/connection_options.cpp
##########
@@ -175,7 +175,8 @@ class connection_options::impl {
}
} else if (!client && ssl_server_options.set) {
pn_ssl_t *ssl = pn_ssl(pnt);
- if (pn_ssl_init(ssl, ssl_server_options.value.impl_->pn_domain(),
NULL)) {
+ pn_ssl_domain_t* ssl_domain = ssl_server_options.value.impl_ ?
ssl_server_options.value.impl_->pn_domain() : NULL;
+ if (pn_ssl_init(ssl, ssl_domain, NULL)) {
Review comment:
It is a shame that this wasn't picked up by any tests - I thought we had
some ssl server tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> [cpp] Performance regression found in 0.29.0
> --------------------------------------------
>
> Key: PROTON-2137
> URL: https://issues.apache.org/jira/browse/PROTON-2137
> Project: Qpid Proton
> Issue Type: Bug
> Components: cpp-binding
> Affects Versions: proton-c-0.29.0
> Reporter: Rabih Mourad
> Priority: Major
> Attachments: code.txt, remove-default-ssl.patch
>
>
> We are upgrading in our Murex code the proton version from 0.27.0 to 0.29.0.
> While running our unit tests, we found a considerable performance regression.
> We were able to reproduce the regression in a very simple use case.
> Please find the code attached.
> This test takes 1 ms in the version 0.27.0 and 0.28.0 but it takes 103 ms in
> 0.29.0 .
>
> After analysis we discovered that the regression is coming from this pull
> request [Proton 2075:[C++] Allow TLS to use system default trusted
> certificate.|https://github.com/apache/qpid-proton/commit/e152190459cd75792002d2aae72d351dc22abe27]
> In fact we noticed that the ssl_client_options and the ssl_server_options
> are not default constructed the same way and that the [ssl_server_options
> |https://github.com/apache/qpid-proton/blob/e152190459cd75792002d2aae72d351dc22abe27/cpp/src/ssl_options.cpp#L99]
> is calling
> [pni_init_ssl_domain|https://github.com/apache/qpid-proton/blob/9dd013335de0694bc52848897b17190f297450c1/c/src/ssl/openssl.c#L475]
> which is taking some time.
> What we would like is to avoid initializing ssl domain when it’s disabled
> from the connection_options.
> Does it sound reasonable for you?
>
> Here is a patch that we propose. All proton unit tests are green on the
> master.
> Can you please [~astitcher] check if this patch is aligned with what you
> meant to do in (PROTON-2075) ?
>
> Thanks,
> Ali & Rabih
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
