Bug 55023 - SSL Context reuse feature (51380) adversely affects non-ssl request performance/throughput
SSL Context reuse feature (51380) adversely affects non-ssl request performan...
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: HTTP
2.9
All All
: P1 regression (vote)
: ---
Assigned To: JMeter issues mailing list
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2013-05-28 22:50 UTC by Brent Cromarty
Modified: 2013-07-21 20:42 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Cromarty 2013-05-28 22:50:30 UTC
The enhancement to control the reuse of cached SSL Context instances between iterations (51380) is causing a performance bottleneck with http (non-ssl) requests.  In HTTPSamplerBase the following code is executed regardless of whether the HTTPS protocol is being used or not:

@Override
public void testIterationStart(LoopIterationEvent event) {
    if (!USE_CACHED_SSL_CONTEXT) {
	JsseSSLManager sslMgr = (JsseSSLManager) SSLManager.getInstance();
	sslMgr.resetContext();
	notifySSLContextWasReset();
    }
}

With JVM profiling enabled on the JMeter process (adding: "-agentlib:hprof=cpu=samples,interval=2,depth=10,monitor=y,thread=n" to the JVM command line) I noticed that even though I was running a script using only HTTP requests (no SSL in the mix), hot spots were identified in SSL code paths:

TRACE 300823:
sun.security.provider.X509Factory.getFromCache(X509Factory.java:203)
sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:93)
java.security.cert.CertificateFactory.generateCertificate(CertificateFactory.java:339)
sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:747)
sun.security.provider.JavaKeyStore$JKS.engineLoad(JavaKeyStore.java:55)
java.security.KeyStore.load(KeyStore.java:1214)
sun.security.ssl.TrustManagerFactoryImpl.getCacertsKeyStore(TrustManagerFactoryImpl.java:221)
sun.security.ssl.TrustManagerFactoryImpl.engineInit(TrustManagerFactoryImpl.java:51)
javax.net.ssl.TrustManagerFactory.init(TrustManagerFactory.java:250)
sun.security.ssl.SSLContextImpl.engineInit(SSLContextImpl.java:83)

TRACE 300844:
java.io.FileInputStream.read(FileInputStream.java:Unknown line)
java.io.DataInputStream.readInt(DataInputStream.java:387)
sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:667)
sun.security.provider.JavaKeyStore$JKS.engineLoad(JavaKeyStore.java:55)
java.security.KeyStore.load(KeyStore.java:1214)
sun.security.ssl.TrustManagerFactoryImpl.getCacertsKeyStore(TrustManagerFactoryImpl.java:221)
sun.security.ssl.TrustManagerFactoryImpl.engineInit(TrustManagerFactoryImpl.java:51)
javax.net.ssl.TrustManagerFactory.init(TrustManagerFactory.java:250)
sun.security.ssl.SSLContextImpl.engineInit(SSLContextImpl.java:83)
javax.net.ssl.SSLContext.init(SSLContext.java:283)

Moreover, when the SSL Context Reset code is executed, in the Apache HTTPClient sampler Implementations, open connections are closed even if they are not established with SSL (HTTPHC4ClientImpl.closeThreadLocalConnections()) thus making "Keep Alive" ineffective.

As a simple fix I added a check on the protocol in use like the following:

@Override
public void testIterationStart(LoopIterationEvent event) {
    if (!USE_CACHED_SSL_CONTEXT &&
        getProtocol().equalsIgnoreCase(HTTPConstants.PROTOCOL_HTTPS)) {
	JsseSSLManager sslMgr = (JsseSSLManager) SSLManager.getInstance();
	sslMgr.resetContext();
	notifySSLContextWasReset();
    }
}

With keep-alive enabled HTTP request throughput increases by around 500% in my environment.  I'm not certain, however, if this is the "most correct" fix because it is based on whether the last request executed in the iteration is SSL or not.
Comment 1 Sebb 2013-05-29 01:05:34 UTC
Thanks for the report and analysis.

Each HTTP sampler has its own HttpSamplerBase instance, so provided that the protocol is a fixed string, the check you propose is fine. For a dynamic protocol it might depend how the protocol was defined - perhaps the value might not be correctly defined at test iteration start [would need checking].

However as you further point out, the HC3/HC4 implementations clear all current connections in the overridden notifySSLContextWasReset() methods. I assume this is because the context is bound to the connection somehow. Ideally one would like to only clear the relevant entry(s). This would be tricky to do at test iteration start, since the connection map key might rely on dynamic values.

So a possible solution might be to set a flag in the testIterationStart() method if the context is to be reset. The first sample that uses SSL would then ignore any cached connection, and always recreate it. And then clear the flag. I think this should solve both issues.
Comment 2 Sebb 2013-06-03 21:41:43 UTC
I moved the testIterationStart code to the samplers where the SSL context needs to be reset. This means the code should only now run if there is an HTTPS sample in the test plan.

If the problem has not been fixed in versions from r1489189 please re-open with full details.

URL: http://svn.apache.org/r1489189
Log:
SSL Context reuse feature (51380) adversely affects non-ssl request performance/throughput
Move testIterationStart code to samplers that actually need it; allows it to be done on first actual sample
Bugzilla Id: 55023

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHCAbstractImpl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSampler2.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
    jmeter/trunk/xdocs/changes.xml
Comment 3 Philippe Mouawad 2013-06-04 20:57:33 UTC
Date: Tue Jun  4 20:56:41 2013
New Revision: 1489603

URL: http://svn.apache.org/r1489603
Log:
Bug 55023 - SSL Context reuse feature (51380) adversely affects non-ssl request performance/throughput
Fixed regression on 51380 introduced by fix
Bugzilla Id: 55023

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSampler2.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
    jmeter/trunk/test/src/org/apache/jmeter/protocol/http/sampler/HTTPSampler3.java
Comment 4 Philippe Mouawad 2013-06-07 07:37:47 UTC
Date: Thu Jun  6 13:43:02 2013
New Revision: 1490281

URL: http://svn.apache.org/r1490281
Log:
Fix bug introduced in http://svn.apache.org/r1489603
The testIterationStart method is called from a different thread from any of the samples

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
Comment 5 Philippe Mouawad 2013-06-07 07:38:08 UTC
Date: Fri Jun  7 07:37:21 2013
New Revision: 1490528

URL: http://svn.apache.org/r1490528
Log:
Bug 55023 - SSL Context reuse feature (51380) adversely affects non-ssl request performance/throughput
Fixed regression on 52310 introduced by fix
- http://svn.apache.org/r1489603
Fixed regression on 51380 introduced by fix
http://svn.apache.org/r1490281
Bugzilla Id: 55023

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java
Comment 6 Brent Cromarty 2013-06-17 22:11:22 UTC
Tested in my environment with developer nightly build (r1493407) and performance for HTTP requests are up where I expect them to be.

Thanks for the quick turnaround on this.  My apologies for not getting to validating the fix sooner.

b
Comment 7 Philippe Mouawad 2013-07-21 19:35:14 UTC
There is a regression introduced by the fix to this bug.
Setting implementation in Http Request Defaults does not work anymore, if no property is set then HttpHcImpl4 will always be used.

This is due to the mergeIn being called after testIterationStart(), so implementation is not set yet.
Comment 8 Philippe Mouawad 2013-07-21 20:42:38 UTC
Date: Sun Jul 21 20:41:43 2013
New Revision: 1505471

URL: http://svn.apache.org/r1505471
Log:
Bug 55023 - SSL Context reuse feature (51380) adversely affects non-ssl request performance/throughput
Bugzilla Id: 55023

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPAbstractImpl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSampler2.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java