[GitHub] [jmeter] vlsi commented on pull request #603: Bug 64558 - Improve performances and throughput of Sample Results by lifting contention on writing SampleResults in CSV/XML

[GitBox <gi...@apache.org>]

vlsi commented on pull request #603:
URL: https://github.com/apache/jmeter/pull/603#issuecomment-654053171


   My concerns are:
   1) "marker interfaces". The pattern was invented long ago when no other means were possible. Now we can have annotations and other means that enable better metadata tracking. Marker interfaces do make the maintenance more complicated, and it not clear why do you add interfaces to "sample listeners" only. For instance, do you intend to create "JMeterThreadBound..." for every JMeter component class type? Then we'll end up in 100 useless interfaces.
   
   Then, as we move to async-based test plan execution, the notion of `JMeterThreadBound` would be even mooter.
   
   2) The test you use looks contrived. Does the change produce noticeable improvement for, say, HTTP testing?
   
   I see you observe improvements like 1M -> 1.7M requests/sec when doing no-op test. Ok. What is the number of requests per second you can achieve on the same machine with a single HTTP sampler (or HTTP + regexp extractor)? Is it 10K/sec?
   Then the maximum possible rate after the improvement would be `1/(1/10K-1/(1.7M-1M))` == 10.1K/sec. Of course, I might be wrong, but it would be surprising if the end-users would observe more than say 2-5% improvement.
   
   Of course, we can always rework or even undo the change, however, adding interfaces like `JMeterThreadBoundSampleListener` to the public API  looks super weird.


----------------------------------------------------------------
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:
users@infra.apache.org