Re: [Qpid-proton-cpp] Performance regression found in 0.29.0

[Rabih M]

Hi,

Jira issue created: PROTON-2137
.

Best regards,
Rabih

On Wed, Nov 20, 2019 at 5:01 PM Andrew Stitcher 
wrote:

> Please raise a JIRA with all this information and the reproducer - it
> is very hard to track fixes without this.
>
> Also you could attach your patch, but it is easier in this case to use
> a github PR with a reference to the JIRA. The Apache automation will
> tie them together and this makes it again easier to track.
>
> Andrew
>
> On Tue, 2019-11-19 at 18:04 +, HADI Ali wrote:
> > Hello,
> >
> > After analysis we discovered that the regression is coming from
> > PROTON-2075<
> >
> https://github.com/apache/qpid-proton/commit/e152190459cd75792002d2aae72d351dc22abe27
> >
> > ;: [C++] Allow TLS to use system default trusted certificate.
> > In fact we noticed that the ssl_client_options and the
> > ssl_server_options are not default constructed the same way and that
> > the second one<
> >
> 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 when it’s disabled
> > from the connection_options.
> > Does it sound reasonable for you? Should we create a Jira issue and
> > propose a fix?
> >
> > Thanks,
> > Ali & Rabih
> >
> > From: Rabih M 
> > Sent: mercredi 13 novembre 2019 19:22
> > To: users@qpid.apache.org
> > Subject: [Qpid-proton-cpp] Performance regression found in 0.29.0
> >
> > Hello,
> >
> > We are upgrading in our 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 73
> > ms in 0.29.0 .
> >
> > Do you know what might be the cause?
> > We will try to investigate in parallel from our side, too.
> >
> > Thanks,
> > Rabih & Ali
> > ***
> > This e-mail contains information for the intended recipient only. It
> > may contain proprietary material or confidential information. If you
> > are not the intended recipient you are not authorized to distribute,
> > copy or use this e-mail or any attachment to it. Murex cannot
> > guarantee that it is virus free and accepts no responsibility for any
> > loss or damage arising from its use. If you have received this e-mail
> > in error please notify immediately the sender and delete the original
> > email received, any attachments and all copies from your system.
>
>
> -
> To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org
> For additional commands, e-mail: users-h...@qpid.apache.org
>
>

Re: [Qpid-proton-cpp] Performance regression found in 0.29.0

[Andrew Stitcher]

Please raise a JIRA with all this information and the reproducer - it
is very hard to track fixes without this.

Also you could attach your patch, but it is easier in this case to use
a github PR with a reference to the JIRA. The Apache automation will
tie them together and this makes it again easier to track.

Andrew

On Tue, 2019-11-19 at 18:04 +, HADI Ali wrote:
> Hello,
> 
> After analysis we discovered that the regression is coming from
> PROTON-2075<
> https://github.com/apache/qpid-proton/commit/e152190459cd75792002d2aae72d351dc22abe27>
> ;: [C++] Allow TLS to use system default trusted certificate.
> In fact we noticed that the ssl_client_options and the
> ssl_server_options are not default constructed the same way and that
> the second one<
> 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 when it’s disabled
> from the connection_options.
> Does it sound reasonable for you? Should we create a Jira issue and
> propose a fix?
> 
> Thanks,
> Ali & Rabih
> 
> From: Rabih M 
> Sent: mercredi 13 novembre 2019 19:22
> To: users@qpid.apache.org
> Subject: [Qpid-proton-cpp] Performance regression found in 0.29.0
> 
> Hello,
> 
> We are upgrading in our 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 73
> ms in 0.29.0 .
> 
> Do you know what might be the cause?
> We will try to investigate in parallel from our side, too.
> 
> Thanks,
> Rabih & Ali
> ***
> This e-mail contains information for the intended recipient only. It
> may contain proprietary material or confidential information. If you
> are not the intended recipient you are not authorized to distribute,
> copy or use this e-mail or any attachment to it. Murex cannot
> guarantee that it is virus free and accepts no responsibility for any
> loss or damage arising from its use. If you have received this e-mail 
> in error please notify immediately the sender and delete the original
> email received, any attachments and all copies from your system.


-
To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org
For additional commands, e-mail: users-h...@qpid.apache.org

Re: [Qpid-proton-cpp] Performance regression found in 0.29.0

[Rabih M]

Hello again,

Here is a patch that we propose. All unit tests are green on the master.

Can you please @Andrew Stitcher check if this patch is aligned with what
you meant to do in (PROTON-2075) ?

Best regards,
Rabih & Ali

On Tue, Nov 19, 2019 at 7:04 PM HADI Ali  wrote:

> Hello,
>
> After analysis we discovered that the regression is coming from
> PROTON-2075<
> https://github.com/apache/qpid-proton/commit/e152190459cd75792002d2aae72d351dc22abe27>:
> [C++] Allow TLS to use system default trusted certificate.
> In fact we noticed that the ssl_client_options and the ssl_server_options
> are not default constructed the same way and that the second one<
> 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 when it’s disabled from
> the connection_options.
> Does it sound reasonable for you? Should we create a Jira issue and
> propose a fix?
>
> Thanks,
> Ali & Rabih
>
> From: Rabih M 
> Sent: mercredi 13 novembre 2019 19:22
> To: users@qpid.apache.org
> Subject: [Qpid-proton-cpp] Performance regression found in 0.29.0
>
> Hello,
>
> We are upgrading in our 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 73 ms
> in 0.29.0 .
>
> Do you know what might be the cause?
> We will try to investigate in parallel from our side, too.
>
> Thanks,
> Rabih & Ali
> ***
> This e-mail contains information for the intended recipient only. It may
> contain proprietary material or confidential information. If you are not
> the intended recipient you are not authorized to distribute, copy or use
> this e-mail or any attachment to it. Murex cannot guarantee that it is
> virus free and accepts no responsibility for any loss or damage arising
> from its use. If you have received this e-mail in error please notify
> immediately the sender and delete the original email received, any
> attachments and all copies from your system.
>
diff --git a/cpp/src/connection_options.cpp b/cpp/src/connection_options.cpp
index 2bf281d1..b61c2c68 100644
--- a/cpp/src/connection_options.cpp
+++ b/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)) {
 throw error(MSG("server SSL/TLS initialization error"));
 }
 }
diff --git a/cpp/src/ssl_options.cpp b/cpp/src/ssl_options.cpp
index bd4d5c17..f74f014e 100644
--- a/cpp/src/ssl_options.cpp
+++ b/cpp/src/ssl_options.cpp
@@ -99,7 +99,7 @@ ssl_server_options::ssl_server_options(
 throw error(MSG("SSL server configuration failure requiring client 
certificates using " << db));
 }
 
-ssl_server_options::ssl_server_options() : impl_(new impl) {}
+ssl_server_options::ssl_server_options() : impl_(0) {}
 
 ssl_client_options::ssl_client_options(const ssl_client_options& x): 
impl_(x.impl_) {
 if (impl_) impl_->incref();

-
To unsubscribe, e-mail: users-unsubscr...@qpid.apache.org
For additional commands, e-mail: users-h...@qpid.apache.org

RE: [Qpid-proton-cpp] Performance regression found in 0.29.0

[HADI Ali]

Hello,

After analysis we discovered that the regression is coming from 
PROTON-2075:
 [C++] Allow TLS to use system default trusted certificate.
In fact we noticed that the ssl_client_options and the ssl_server_options are 
not default constructed the same way and that the second 
one
 is calling 
pni_init_ssl_domain
 which is taking some time.

What we would like is to avoid initializing ssl when it’s disabled from the 
connection_options.
Does it sound reasonable for you? Should we create a Jira issue and propose a 
fix?

Thanks,
Ali & Rabih

From: Rabih M 
Sent: mercredi 13 novembre 2019 19:22
To: users@qpid.apache.org
Subject: [Qpid-proton-cpp] Performance regression found in 0.29.0

Hello,

We are upgrading in our 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 73 ms in 
0.29.0 .

Do you know what might be the cause?
We will try to investigate in parallel from our side, too.

Thanks,
Rabih & Ali
***
This e-mail contains information for the intended recipient only. It may 
contain proprietary material or confidential information. If you are not the 
intended recipient you are not authorized to distribute, copy or use this 
e-mail or any attachment to it. Murex cannot guarantee that it is virus free 
and accepts no responsibility for any loss or damage arising from its use. If 
you have received this e-mail in error please notify immediately the sender and 
delete the original email received, any attachments and all copies from your 
system.