You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Bernd Eckenfels <ec...@zusammenkunft.net> on 2014/09/20 07:50:13 UTC
[Pool] Performance Tests for Pool-277
Hello,
re https://issues.apache.org/jira/browse/POOL-277
I would like to commit VFS-277 fix (nonlockstats2.patch). However before
I do that, I would like to test if it is really a perfomrmance
improvement. Lucas confirms it helps in his scenario.
I have run the included PerformanceTest, but the results have been
indifferent. Are those tests reliable? Can somebody with some
experience re-run them on their machine? (I somewhat feel the need to
provide a JMH test :)
My proposed patch:
https://issues.apache.org/jira/secure/attachment/12670185/nolockstats2.patch
Gruss
Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Phil Steitz <ph...@gmail.com>.
On 9/21/14 10:30 AM, Phil Steitz wrote:
> On 9/19/14 10:50 PM, Bernd Eckenfels wrote:
>> Hello,
>>
>> re https://issues.apache.org/jira/browse/POOL-277
>>
>> I would like to commit VFS-277 fix (nonlockstats2.patch). However before
>> I do that, I would like to test if it is really a perfomrmance
>> improvement. Lucas confirms it helps in his scenario.
>>
>> I have run the included PerformanceTest, but the results have been
>> indifferent. Are those tests reliable? Can somebody with some
>> experience re-run them on their machine? (I somewhat feel the need to
>> provide a JMH test :)
> I have started running some performance tests using commons
> performance (sandbox). I mostly use [performance] to get races or
> other bugs to happen, but it does give an idea of gross performance
> differences. So far, I have not been able to detect any performance
> benefit from the patch. That does not mean that there is no
> benefit, especially since I have only 4 cores on the test box. I
> have some longer-running tests running now and will report back if I
> find anything.
>
> Even if the patch does improve performance, I am -0 on applying as
> is, as the implementation appears to change the meaning of
> getMaxBorrowWaitTimeMillis. The current implementation returns the
> max since the pool was created, not a rolling window of recent
> values, as the patch appears to do (unless I am missing something).
> The unit tests for reported JMX stats are pretty weak, so we need to
> be careful relying on them to confirm correctness.
Looking more carefully at the patch, I think what I stated above is
incorrect - i.e., the global max should be returned, so that is not
a blocker here. Sorry I misunderstood the code. Now we need to
confirm that there is in fact a performance benefit. Also, we
should look at the impact on the getters performance - I suspect
they will be a little slower.
Phil
>
> Phil
>> My proposed patch:
>> https://issues.apache.org/jira/secure/attachment/12670185/nolockstats2.patch
>>
>> Gruss
>> Bernd
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Phil Steitz <ph...@gmail.com>.
On 9/24/14 12:20 AM, Mark Thomas wrote:
> On 24/09/2014 00:51, Bernd Eckenfels wrote:
>> Hello Mark,
>>
>> I did some micro benchmarks on the max-only thing. And they somewhat
>> look like I have expected on my 4core/8threads:
>>
>> 1 thread
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 71754699,465 4916943,615 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 31398785,726 1071751,697 ops/s
>>
>> 2 threads
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 111773939,071 6478325,697 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 8197066,038 457199,732 ops/s
>>
>> 4 threads
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 190326272,383 11287614,702 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5227629,001 35488,823 ops/s
>>
>> 6 threads
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 257831969,334 21506920,317 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5437590,131 651603,805 ops/s
>>
>> 8 threads
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 308313078,989 7365995,913 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4808905,027 657214,727 ops/s
>>
>> 16 threads
>> Benchmark Mode Samples Score Score error Units
>> n.e.j.t.MyBenchmark.atomicMax thrpt 5 295064556,539 28027347,836 ops/s
>> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4974286,841 79612,105 ops/s
>>
>>
>> I would say one sees that both get slower when the active conurrency gets higher, but the atomic version is always faster. When there are more threads than cores it even gets a bit better.
>>
>> So I would say it is safe to only replace the synchronized max version with the atomic one. The further changes like moving it to the stats store and cleaning up the mean calculation and the synchronized method can be left for later:
>>
>> I would actually like to introduce a StatsStore interface where the user can register their own implementaions. They would have to provide add() and getMean() and getMax() but they can query all they want from their smart implementation (including percentils and stuff).
>>
>> Until then, I attached a more minimal patch for the atomicMax of the borrow (only).
> Based on the above, I have no objections to either the minimal patch or
> the current patch.
+1
Phil
>
> Mark
>
>
>> Gruss
>> Bernd
>>
>> PS: code is here: https://gist.github.com/ecki/8350a276caa444a62dce
>>
>>
>>
>>
>> Am Tue, 23 Sep 2014 19:11:42 +0100
>> schrieb Mark Thomas <ma...@apache.org>:
>>
>>> On 23/09/2014 17:59, Phil Steitz wrote:
>>>> On 9/21/14 7:29 PM, Phil Steitz wrote:
>>> <snip/>
>>>
>>>>> I was not able to demonstrate any consistent performance difference
>>>>> in my tests. The code in trunk was a tiny bit faster on average
>>>>> but some runs of the patched code were faster than some runs of the
>>>>> unpatched code. It might be worth just changing the max to be
>>>>> "lock free" and seeing if that change by itself makes any
>>>>> difference.
>>>> I did that and still could not demonstrate any improvement. The
>>>> report in the ticket does show monitor contention, though, so I am
>>>> ambivalent on this one. What do others think?
>>> Typo: s/Mast/Must/
>>>
>>> I have often found that for large numbers of cheap operations that
>>> monitoring tools are far more overhead than the thing they are trying
>>> to measure. I recently spent some time tuning a new Cookie parser for
>>> Tomcat and the profiler results were useless at identifying the real
>>> bottlenecks. I wonder if that is happening here.
>>>
>>> I have no objection to the patch if the performance is about the same.
>>> Adding max to the StatsStore is likely to be useful elsewhere.
>>>
>>> Mark
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Mark Thomas <ma...@apache.org>.
On 24/09/2014 00:51, Bernd Eckenfels wrote:
> Hello Mark,
>
> I did some micro benchmarks on the max-only thing. And they somewhat
> look like I have expected on my 4core/8threads:
>
> 1 thread
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 71754699,465 4916943,615 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 31398785,726 1071751,697 ops/s
>
> 2 threads
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 111773939,071 6478325,697 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 8197066,038 457199,732 ops/s
>
> 4 threads
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 190326272,383 11287614,702 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5227629,001 35488,823 ops/s
>
> 6 threads
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 257831969,334 21506920,317 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5437590,131 651603,805 ops/s
>
> 8 threads
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 308313078,989 7365995,913 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4808905,027 657214,727 ops/s
>
> 16 threads
> Benchmark Mode Samples Score Score error Units
> n.e.j.t.MyBenchmark.atomicMax thrpt 5 295064556,539 28027347,836 ops/s
> n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4974286,841 79612,105 ops/s
>
>
> I would say one sees that both get slower when the active conurrency gets higher, but the atomic version is always faster. When there are more threads than cores it even gets a bit better.
>
> So I would say it is safe to only replace the synchronized max version with the atomic one. The further changes like moving it to the stats store and cleaning up the mean calculation and the synchronized method can be left for later:
>
> I would actually like to introduce a StatsStore interface where the user can register their own implementaions. They would have to provide add() and getMean() and getMax() but they can query all they want from their smart implementation (including percentils and stuff).
>
> Until then, I attached a more minimal patch for the atomicMax of the borrow (only).
Based on the above, I have no objections to either the minimal patch or
the current patch.
Mark
>
> Gruss
> Bernd
>
> PS: code is here: https://gist.github.com/ecki/8350a276caa444a62dce
>
>
>
>
> Am Tue, 23 Sep 2014 19:11:42 +0100
> schrieb Mark Thomas <ma...@apache.org>:
>
>> On 23/09/2014 17:59, Phil Steitz wrote:
>>> On 9/21/14 7:29 PM, Phil Steitz wrote:
>>
>> <snip/>
>>
>>>> I was not able to demonstrate any consistent performance difference
>>>> in my tests. The code in trunk was a tiny bit faster on average
>>>> but some runs of the patched code were faster than some runs of the
>>>> unpatched code. It might be worth just changing the max to be
>>>> "lock free" and seeing if that change by itself makes any
>>>> difference.
>>>
>>> I did that and still could not demonstrate any improvement. The
>>> report in the ticket does show monitor contention, though, so I am
>>> ambivalent on this one. What do others think?
>>
>> Typo: s/Mast/Must/
>>
>> I have often found that for large numbers of cheap operations that
>> monitoring tools are far more overhead than the thing they are trying
>> to measure. I recently spent some time tuning a new Cookie parser for
>> Tomcat and the profiler results were useless at identifying the real
>> bottlenecks. I wonder if that is happening here.
>>
>> I have no objection to the patch if the performance is about the same.
>> Adding max to the StatsStore is likely to be useful elsewhere.
>>
>> Mark
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Bernd Eckenfels <ec...@zusammenkunft.net>.
Hello Mark,
I did some micro benchmarks on the max-only thing. And they somewhat
look like I have expected on my 4core/8threads:
1 thread
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 71754699,465 4916943,615 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 31398785,726 1071751,697 ops/s
2 threads
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 111773939,071 6478325,697 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 8197066,038 457199,732 ops/s
4 threads
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 190326272,383 11287614,702 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5227629,001 35488,823 ops/s
6 threads
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 257831969,334 21506920,317 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 5437590,131 651603,805 ops/s
8 threads
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 308313078,989 7365995,913 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4808905,027 657214,727 ops/s
16 threads
Benchmark Mode Samples Score Score error Units
n.e.j.t.MyBenchmark.atomicMax thrpt 5 295064556,539 28027347,836 ops/s
n.e.j.t.MyBenchmark.synchronizedMax thrpt 5 4974286,841 79612,105 ops/s
I would say one sees that both get slower when the active conurrency gets higher, but the atomic version is always faster. When there are more threads than cores it even gets a bit better.
So I would say it is safe to only replace the synchronized max version with the atomic one. The further changes like moving it to the stats store and cleaning up the mean calculation and the synchronized method can be left for later:
I would actually like to introduce a StatsStore interface where the user can register their own implementaions. They would have to provide add() and getMean() and getMax() but they can query all they want from their smart implementation (including percentils and stuff).
Until then, I attached a more minimal patch for the atomicMax of the borrow (only).
Gruss
Bernd
PS: code is here: https://gist.github.com/ecki/8350a276caa444a62dce
Am Tue, 23 Sep 2014 19:11:42 +0100
schrieb Mark Thomas <ma...@apache.org>:
> On 23/09/2014 17:59, Phil Steitz wrote:
> > On 9/21/14 7:29 PM, Phil Steitz wrote:
>
> <snip/>
>
> >> I was not able to demonstrate any consistent performance difference
> >> in my tests. The code in trunk was a tiny bit faster on average
> >> but some runs of the patched code were faster than some runs of the
> >> unpatched code. It might be worth just changing the max to be
> >> "lock free" and seeing if that change by itself makes any
> >> difference.
> >
> > I did that and still could not demonstrate any improvement. The
> > report in the ticket does show monitor contention, though, so I am
> > ambivalent on this one. What do others think?
>
> Typo: s/Mast/Must/
>
> I have often found that for large numbers of cheap operations that
> monitoring tools are far more overhead than the thing they are trying
> to measure. I recently spent some time tuning a new Cookie parser for
> Tomcat and the profiler results were useless at identifying the real
> bottlenecks. I wonder if that is happening here.
>
> I have no objection to the patch if the performance is about the same.
> Adding max to the StatsStore is likely to be useful elsewhere.
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Mark Thomas <ma...@apache.org>.
On 23/09/2014 17:59, Phil Steitz wrote:
> On 9/21/14 7:29 PM, Phil Steitz wrote:
<snip/>
>> I was not able to demonstrate any consistent performance difference
>> in my tests. The code in trunk was a tiny bit faster on average but
>> some runs of the patched code were faster than some runs of the
>> unpatched code. It might be worth just changing the max to be "lock
>> free" and seeing if that change by itself makes any difference.
>
> I did that and still could not demonstrate any improvement. The
> report in the ticket does show monitor contention, though, so I am
> ambivalent on this one. What do others think?
Typo: s/Mast/Must/
I have often found that for large numbers of cheap operations that
monitoring tools are far more overhead than the thing they are trying to
measure. I recently spent some time tuning a new Cookie parser for
Tomcat and the profiler results were useless at identifying the real
bottlenecks. I wonder if that is happening here.
I have no objection to the patch if the performance is about the same.
Adding max to the StatsStore is likely to be useful elsewhere.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Phil Steitz <ph...@gmail.com>.
On 9/21/14 7:29 PM, Phil Steitz wrote:
> On 9/21/14 4:51 PM, Bernd Eckenfels wrote:
>> Hello Phil, thanks for taking a look.
>>
>> See my further discussion inline.
>>
>> Am Sun, 21 Sep 2014 10:30:49 -0700
>> schrieb Phil Steitz <ph...@gmail.com>:
>>
>>> On 9/19/14 10:50 PM, Bernd Eckenfels wrote:
>>>> re https://issues.apache.org/jira/browse/POOL-277
>>>>
>>>> I would like to commit VFS-277 fix (nonlockstats2.patch). However
>>>> before I do that, I would like to test if it is really a
>>>> perfomrmance improvement. Lucas confirms it helps in his scenario.
>>>>
>>>> I have run the included PerformanceTest, but the results have been
>>>> indifferent. Are those tests reliable? Can somebody with some
>>>> experience re-run them on their machine? (I somewhat feel the need
>>>> to provide a JMH test :)
>>> I have started running some performance tests using commons
>>> performance (sandbox). I mostly use [performance] to get races or
>>> other bugs to happen, but it does give an idea of gross performance
>>> differences. So far, I have not been able to detect any performance
>>> benefit from the patch. That does not mean that there is no
>>> benefit, especially since I have only 4 cores on the test box. I
>>> have some longer-running tests running now and will report back if I
>>> find anything.
>> The original reporter reported, that it solved his congestion. But I am
>> also not quite sure what caused the congestion in the first place, and
>> - if there is no contention on the locked section anymore - actually
>> this is better or only a result of beeing slow in other places. I do
>> think the statisticd object is rather lass content in real world
>> scenarios involving external resources.
>>
>>> Even if the patch does improve performance, I am -0 on applying as
>>> is, as the implementation appears to change the meaning of
>>> getMaxBorrowWaitTimeMillis.
>> Actually I think the old meaning was totally broken. It first of all
>> implements some expotential average algorithm without the need for it
>> (as it keeps a lot of samples anyway). And secondly it implements it
>> wrong, as it always starts from index 0, which is not necesarily the
>> right (oldest) index. Both - the old and the new version - are a
>> ringbuffer with a varying start.
>>
>>> The current implementation returns the
>>> max since the pool was created, not a rolling window of recent
>>> values, as the patch appears to do (unless I am missing something).
>>> The unit tests for reported JMX stats are pretty weak, so we need to
>>> be careful relying on them to confirm correctness.
>> I think those are two things, the "max" is in both cases from the
>> start, and the rolling window is supposed to be of the last 100(128)
>> values. (as you have written in your second mail). So from this point
>> of view we are I think safe: yes the calculation will return different
>> results, but the new implementation is more predictable.
> Could be I am misreading the patched code, but I don't think so. I
> think it actually preserves the original behavior - the max is
> global. The other stats are not.
>
>> But I agree we should have a look at the performance some more. We
>> might revert the add() optimizations and only keep the "maximum" lock
>> free implementation. It was reported to help (and again I am not sure
>> why).
>>
>> BTW: I am not entirely sure if we need the Atomic(Array) for the
>> history of last values. I wonder if a volatile array might work as
>> well: it is not entirely correct but as it is only used to collect
>> historical data and it is unlikkely it can and will keep a whole array
>> buffered it might be enugh to go with this (especially since the index
>> is an atomic).
>>
>> The current code is not so easy to test, but I guess I will just copy
>> it into some JMH tests to do some performance testings for the various
>> aspects of getters and setters.
> I was not able to demonstrate any consistent performance difference
> in my tests. The code in trunk was a tiny bit faster on average but
> some runs of the patched code were faster than some runs of the
> unpatched code. It might be worth just changing the max to be "lock
> free" and seeing if that change by itself makes any difference.
I did that and still could not demonstrate any improvement. The
report in the ticket does show monitor contention, though, so I am
ambivalent on this one. What do others think?
Phil
>
>
> Phil
>> Gruss
>> Bernd
>>
>>
>>> Phil
>>>> My proposed patch:
>>>> https://issues.apache.org/jira/secure/attachment/12670185/nolockstats2.patch
>>>>
>>>> Gruss
>>>> Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Bernd Eckenfels <ec...@zusammenkunft.net>.
Hello Phil, thanks for taking a look.
See my further discussion inline.
Am Sun, 21 Sep 2014 10:30:49 -0700
schrieb Phil Steitz <ph...@gmail.com>:
> On 9/19/14 10:50 PM, Bernd Eckenfels wrote:
> > re https://issues.apache.org/jira/browse/POOL-277
> >
> > I would like to commit VFS-277 fix (nonlockstats2.patch). However
> > before I do that, I would like to test if it is really a
> > perfomrmance improvement. Lucas confirms it helps in his scenario.
> >
> > I have run the included PerformanceTest, but the results have been
> > indifferent. Are those tests reliable? Can somebody with some
> > experience re-run them on their machine? (I somewhat feel the need
> > to provide a JMH test :)
>
> I have started running some performance tests using commons
> performance (sandbox). I mostly use [performance] to get races or
> other bugs to happen, but it does give an idea of gross performance
> differences. So far, I have not been able to detect any performance
> benefit from the patch. That does not mean that there is no
> benefit, especially since I have only 4 cores on the test box. I
> have some longer-running tests running now and will report back if I
> find anything.
The original reporter reported, that it solved his congestion. But I am
also not quite sure what caused the congestion in the first place, and
- if there is no contention on the locked section anymore - actually
this is better or only a result of beeing slow in other places. I do
think the statisticd object is rather lass content in real world
scenarios involving external resources.
> Even if the patch does improve performance, I am -0 on applying as
> is, as the implementation appears to change the meaning of
> getMaxBorrowWaitTimeMillis.
Actually I think the old meaning was totally broken. It first of all
implements some expotential average algorithm without the need for it
(as it keeps a lot of samples anyway). And secondly it implements it
wrong, as it always starts from index 0, which is not necesarily the
right (oldest) index. Both - the old and the new version - are a
ringbuffer with a varying start.
> The current implementation returns the
> max since the pool was created, not a rolling window of recent
> values, as the patch appears to do (unless I am missing something).
> The unit tests for reported JMX stats are pretty weak, so we need to
> be careful relying on them to confirm correctness.
I think those are two things, the "max" is in both cases from the
start, and the rolling window is supposed to be of the last 100(128)
values. (as you have written in your second mail). So from this point
of view we are I think safe: yes the calculation will return different
results, but the new implementation is more predictable.
But I agree we should have a look at the performance some more. We
might revert the add() optimizations and only keep the "maximum" lock
free implementation. It was reported to help (and again I am not sure
why).
BTW: I am not entirely sure if we need the Atomic(Array) for the
history of last values. I wonder if a volatile array might work as
well: it is not entirely correct but as it is only used to collect
historical data and it is unlikkely it can and will keep a whole array
buffered it might be enugh to go with this (especially since the index
is an atomic).
The current code is not so easy to test, but I guess I will just copy
it into some JMH tests to do some performance testings for the various
aspects of getters and setters.
Gruss
Bernd
> Phil
> > My proposed patch:
> > https://issues.apache.org/jira/secure/attachment/12670185/nolockstats2.patch
> >
> > Gruss
> > Bernd
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Pool] Performance Tests for Pool-277
Posted by Phil Steitz <ph...@gmail.com>.
On 9/19/14 10:50 PM, Bernd Eckenfels wrote:
> Hello,
>
> re https://issues.apache.org/jira/browse/POOL-277
>
> I would like to commit VFS-277 fix (nonlockstats2.patch). However before
> I do that, I would like to test if it is really a perfomrmance
> improvement. Lucas confirms it helps in his scenario.
>
> I have run the included PerformanceTest, but the results have been
> indifferent. Are those tests reliable? Can somebody with some
> experience re-run them on their machine? (I somewhat feel the need to
> provide a JMH test :)
I have started running some performance tests using commons
performance (sandbox). I mostly use [performance] to get races or
other bugs to happen, but it does give an idea of gross performance
differences. So far, I have not been able to detect any performance
benefit from the patch. That does not mean that there is no
benefit, especially since I have only 4 cores on the test box. I
have some longer-running tests running now and will report back if I
find anything.
Even if the patch does improve performance, I am -0 on applying as
is, as the implementation appears to change the meaning of
getMaxBorrowWaitTimeMillis. The current implementation returns the
max since the pool was created, not a rolling window of recent
values, as the patch appears to do (unless I am missing something).
The unit tests for reported JMX stats are pretty weak, so we need to
be careful relying on them to confirm correctness.
Phil
>
> My proposed patch:
> https://issues.apache.org/jira/secure/attachment/12670185/nolockstats2.patch
>
> Gruss
> Bernd
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org