RFR: 8009723 CMS logs "concurrent mode failure" twice when using (disabling) -XX:-UseCMSCompactAtFullCollection

[Bengt Rutisson]

Hi Kevin,

Looks good!

I'm fine with pushing this without the test, but I kind of think it is 
better to include the test. If we have a taken the time to write a test 
I think it is a good idea to include it. We really need the test 
framework to allow us to specify how the tests can be run to not waste 
resources. So, I think 3) below is something we are always running in to 
and need fixed. Thus, I don't think that should stop us from adding tests.

If you decide to include the test I would like to review it more 
closely. One thing is that I think it might be simpler to write the test 
using the testlibrary rather than implementing your own process support.

Bengt


On 3/9/13 12:39 AM, Kevin Walls wrote:
> Hi,
>
> This is a review request for a small change:
>
> http://cr.openjdk.java.net/~kevinw/8009723/webrev/
>
> Using CMS and disabling UseCMSCompactAtFullCollection on the command 
> line easily produces a double-logging of the "(concurrent mode 
> failure)" message.  This was spotted by Erik Helin.
>
> This is like removing a print statement.
>
> The webrev contains a testcase, but I am currently suggesting I commit 
> this without the test as:
>
> 1) it's fairly trivial: remove a print statement, you don't the see 
> output of that print statement.
> 2) it requires -XX:-UseCMSCompactAtFullCollection which I don't 
> believe is commonly used
> 3) it is a CMS-specific and the current test framework is quite 
> awkward to make the test specific (manual work in the java test, or 
> re-exec.  If re-execing do we ignore TESTVMOPTS or parse it and add 
> UseConcMarkSweepGC?... a few issues there we should solve but this may 
> not be the test change that needs that...)
>
> So the test is possible, and useful for verifying it if anybody wants 
> to run it right now, but does not seem a necessity and for this small 
> change adding to the test harness library to make it more elegant 
> seems unnecessary for this minor problem...
>
> Thanks
> Kevin
>