pmouawad opened a new pull request #603: URL: https://github.com/apache/jmeter/pull/603
Hello Team, As reported in bugzilla (https://bz.apache.org/bugzilla/show_bug.cgi?id=64558), we have noticed a major contention in JMeter that becomes a major problem at high scale. The issue is related to the PrintWriter in ResultCollector. In tests with high throughput (> 100 req/s), the lock taken by PrintWriter#println() will lead many threads to block waiting for the lock to be released (even with the underlying buffering). Following review done by JMeter team on PR 601 and PR 602 (Thanks Felix ! , Thanks Vladimir !), we noticed important performance enhancement and more stable results using ArrayBlockingQueue. Results with ArrayBlockingQueue show stable performance improvements We notice an increase by 69% of the throughput of this: ` jmeter -n -t test_bug_64558.jmx -Jjmeter.save.queue.size=524288 -Jthreads=2000 -Jduration=120 -Jrampup=10 -l results_buffer_256k_q524288.csv ` vs this one JMeter 5.3: ` jmeter -n -t test_bug_64558.jmx -Jthreads=2000 -Jduration=120 -Jrampup=10 -l results.csv ` Implementation does the following: - Use a Queue within ListenerNotifier (enabled for now to see tests run, otherwise regressions could be missed), it can be disabled by setting size to 0 to revert to existing behaviour. This allows optimization to apply on all listeners, like Summarizer - Introduce 2 marker interfaces JMeterThreadBoundSampleListener / JMeterThreadUnboundSampleListener as in fact some SampleListener MUST be executed within the JMeterThread and cannot be asynchronously executed. - Mark all listeners as one of the 2 interfaces - Make ListenerNotifier a Singleton - Introduce a configurable buffer for output CSV/XML file (property:jmeter.save.saveservice.buffer) => This alone improves slightly performance - Cache properties that are highly requested : Thread Group name, isSuccess, IsError , filename - Remove costly synchronization on some metrics in favor of multiple AtomicInteger, this looks an acceptable trade-off to me, but if needed we can revert it ## Impacts on JMeter Behaviour When Queue is enabled, the order of execution for SampleListener will be: - SampleListener that are not JMeterThreadBoundSampleListener within JMeterThread as currently - SampleListener that are JMeterThreadUnboundSampleListener within asynchronous queue Contributed by UbikLoadPack Team: Florent Benoit Philippe Website: https://ubikloadpack.com Twitter: @ubikloadpack ## How Has This Been Tested? 1) ListenerNotifier queue size to 0: ./gradlew clean test **All tests passing** 2) ListenerNotifier queue size to 5000: ./gradlew clean test **All tests passing** 3) benchmark tests as reported above Throughput improved by 68% Files: [test_bug_64558.jmx.txt](https://github.com/apache/jmeter/files/4840938/test_bug_64558.jmx.txt) ---------------------------------------------------------------- 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]
