You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2015/07/04 15:12:40 UTC

[Bug 58099] New: Lazily initialize HttpClient SSL Context

https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

            Bug ID: 58099
           Summary: Lazily initialize HttpClient SSL Context
           Product: JMeter
           Version: 2.6
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: HTTP
          Assignee: issues@jmeter.apache.org
          Reporter: p.mouawad@ubik-ingenierie.com

Currently HTTPClient 4 implementation eagerly creates an SSLContext , degrading
in a important way performances of HttpClient init.

This is particularly annoying for tests using Concurrent download for resources
and for test that do not use HTTPS at all.


This would be helpful as it would:
- improve overall performance particularly for Concurrent download
- Unblock https://issues.apache.org/bugzilla/show_bug.cgi?id=52073


This issue is created after closing Bug 57320.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #15 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #13)
> Hi Sebb,
> (In reply to Sebb from comment #12)
> > (In reply to Philippe Mouawad from comment #11)
> > > Hi,
> > > No SlowHC4SSLSocketFactory is not useful anymore, look at code it just
> > > extends HC4TrustAllSSLSocketFactory without anything specific.
> > 
> > That's not so; it passes cps to HttpSSLProtocolSocketFactory.
> > If that's not working it needs to be fixed, not arbitrarily abandoned.
> > 
> 
> I am aware of that,look at HC4TrustAllSSLSocketFactory, it takes into
> account CPS also.
> 
> > > I deprecated it , we could even remove it.
> > 
> > In any case, this issue is entirely separate.
> 
> > Removal of SlowHC4SSLSocketFactory would require a separate issue (but I
> > don't think it should be removed).
> 
> No it is not needed as HttpSSLProtocolSocketFactory does its job already
> > 
> > Please revert the parts that removed SlowHC4SSLSocketFactory.
> 
> It is related as this removal occured through implementation of 
> LazySchemeSocketFactory.
> 
> Unless I am missing something, I don't see any issue here

It does seem to work OK.

It seems SlowHC4SSLSocketFactory was effectively made obsolete in Bugzilla
55455.

So it's not suprising that dropping it as a part of this bug is confusing.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |p.mouawad@ubik-ingenierie.c
                   |                            |om

--- Comment #2 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
I don't think workaround works as the httpclient is shared in thread local but
setup thread group threads die before thread group thread start.

The current state impacts uselessly tests that only run http, it drastically
degrades performances of tests that use embedded resources parallel download.

For me this bug should be in our priorities.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #3 from Sebb <se...@apache.org> ---
The work-round seemed to work for me. At least some of the startup overhead was
reduced. I think this is because of the static{} block.

What is the SSL code that is being invoked per thread?
I had a quick look and could not find anything SSL related apart from a call to
JsseSSLManager sslMgr = (JsseSSLManager) SSLManager.getInstance() which is only
done if the protocol is HTTPS.

But perhaps I have missed something.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #7 from Sebb <se...@apache.org> ---
Comment on attachment 33567
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33567
Patch fixing this issue

[New file needs an AL header]

Looks like adaptee is created using double-checked locking.
That is not thread-safe unless protected by volatile (or a full lock, in which
case no point in double-checking).

Given that the adaptee depends only on the value of CPS_HTTPS, and that is only
checked at startup, there will only ever be one adaptee. So the code could
perhaps use the IODH idiom instead.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #14 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Author: pmouawad
Date: Thu Feb 18 23:44:53 2016
New Revision: 1731172

URL: http://svn.apache.org/viewvc?rev=1731172&view=rev
Log:
Bug 58099 - Performance : Lazily initialize HttpClient SSL Context to avoid its
initialization even for HTTP only scenarios
Fix regression on HTTPS tests, No route to host...
Bugzilla Id: 58099

Modified:
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/LazySchemeSocketFactory.java

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Lazily initialize           |Performance : Lazily
                   |HttpClient SSL Context      |initialize HttpClient SSL
                   |                            |Context to avoid its
                   |                            |initialization even for
                   |                            |HTTP only scenarios

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Sebb <se...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |52073

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Hi Sebb,
(In reply to Sebb from comment #12)
> (In reply to Philippe Mouawad from comment #11)
> > Hi,
> > No SlowHC4SSLSocketFactory is not useful anymore, look at code it just
> > extends HC4TrustAllSSLSocketFactory without anything specific.
> 
> That's not so; it passes cps to HttpSSLProtocolSocketFactory.
> If that's not working it needs to be fixed, not arbitrarily abandoned.
> 

I am aware of that,look at HC4TrustAllSSLSocketFactory, it takes into account
CPS also.

> > I deprecated it , we could even remove it.
> 
> In any case, this issue is entirely separate.

> Removal of SlowHC4SSLSocketFactory would require a separate issue (but I
> don't think it should be removed).

No it is not needed as HttpSSLProtocolSocketFactory does its job already
> 
> Please revert the parts that removed SlowHC4SSLSocketFactory.

It is related as this removal occured through implementation of 
LazySchemeSocketFactory.

Unless I am missing something, I don't see any issue here

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #8 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Author: pmouawad
Date: Wed Feb 17 21:31:24 2016
New Revision: 1730947

URL: http://svn.apache.org/viewvc?rev=1730947&view=rev
Log:
Bug 58099 - Performance : Lazily initialize HttpClient SSL Context to avoid its
initialization even for HTTP only scenarios
Bugzilla Id: 58099

Added:
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/LazySchemeSocketFactory.java
  (with props)
Modified:
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/SlowHC4SSLSocketFactory.java
    jmeter/trunk/xdocs/changes.xml

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #16 from Sebb <se...@apache.org> ---
URL: http://svn.apache.org/viewvc?rev=1731180&view=rev
Log:
Performance : Lazily initialize HttpClient SSL Context to avoid its
initialization even for HTTP only scenarios
Simplify by using IODH idiom
Bugzilla Id: 58099

Modified:
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/LazySchemeSocketFactory.java

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #4 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Sebb from comment #3)
> The work-round seemed to work for me. At least some of the startup overhead
> was reduced. I think this is because of the static{} block.
> 
Read the thread on dev list called:

httpClient.getConnectionManager() performance with HTTP only
Oleg wrote this:

-----------------------------------
I see. For the time what you can do is to use a custom SSL socket
factory that lazily initializes SSL context when requested for the first
time. This is exactly what HC 3.1 does. It will be somewhat slower given
that one would need to mutex to synchronize access to the initialization
code

-----------------------------------




> What is the SSL code that is being invoked per thread?
> I had a quick look and could not find anything SSL related apart from a call
> to JsseSSLManager sslMgr = (JsseSSLManager) SSLManager.getInstance() which
> is only done if the protocol is HTTPS.
> 
> But perhaps I have missed something.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #5 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Created attachment 33567
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33567&action=edit
Patch fixing this issue

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Sebb <se...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #12 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #11)
> Hi,
> No SlowHC4SSLSocketFactory is not useful anymore, look at code it just
> extends HC4TrustAllSSLSocketFactory without anything specific.

That's not so; it passes cps to HttpSSLProtocolSocketFactory.
If that's not working it needs to be fixed, not arbitrarily abandoned.

> I deprecated it , we could even remove it.

In any case, this issue is entirely separate.
Removal of SlowHC4SSLSocketFactory would require a separate issue (but I don't
think it should be removed).

Please revert the parts that removed SlowHC4SSLSocketFactory.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #11 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Hi,
No SlowHC4SSLSocketFactory is not useful anymore, look at code it just extends
HC4TrustAllSSLSocketFactory without anything specific.
I deprecated it , we could even remove it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #9 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Sebb from comment #7)
> Comment on attachment 33567 [details]
> Patch fixing this issue
> 
> [New file needs an AL header]
fixed
> 
> Looks like adaptee is created using double-checked locking.
> That is not thread-safe unless protected by volatile (or a full lock, in
> which case no point in double-checking).
> 
Thanks, used volatile.

> Given that the adaptee depends only on the value of CPS_HTTPS, and that is
> only checked at startup, there will only ever be one adaptee. So the code
> could perhaps use the IODH idiom instead.
Didn't do it, feel free to do it if you think it is useful.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #6 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
*** Bug 59020 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Lazily initialize HttpClient SSL Context

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

--- Comment #1 from Sebb <se...@apache.org> ---
Lazy init would work well for tests that don't use HTTPS.

However it would still cause a slight slowdown in tests that use HTTPS when the
first HTTPS sample is performed. (The sample response time is not affected)

A work-round is to add a dummy HTTP request in a setUp thread group.

Note that there will be other initialisation overheads in JMeter.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58099] Performance : Lazily initialize HttpClient SSL Context to avoid its initialization even for HTTP only scenarios

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58099

Sebb <se...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #10 from Sebb <se...@apache.org> ---
When I wrote:

"there will only ever be one adaptee"

I meant that for a given test run, there will only be a single adaptee.

However this may be either a HC4TrustAllSSLSocketFactory or a
SlowHC4SSLSocketFactory

The commit that was applied no longer uses SlowHC4SSLSocketFactory, however
AFAICT it should (as per the diff that was originally posted)

-- 
You are receiving this mail because:
You are the assignee for the bug.