RFR(S): 8150865: SQE test: GC unified logging: check that dynamic log level doesn't break anything

[Jesper Wilhelmsson]

Den 10/5/16 kl. 08:12, skrev Dmitry Fazunenko:
> Hi Kirill,
>
> The test looks very good!
> Some comments which will make it look even better :)
>
> 1) Please remove dependency on G1. Unified logging is a common
> feature and should be tested in all collectors.
> You can keep two kinds of objects: small and large. To avoid confusion,
> "large" or "huge" term could be used instead of "humongous".
>
> 2) Because this is a stress test, please move it: gc/logging --> gc/stress

I assume this is done to make it easier to select tests in different test lists?

In general I would prefer to sort the tests based on what they test, not what 
kind of test it is. It's easier to find the right test when one has made a 
change in some part of the code if all tests related to that code is in the same 
place.

Updating the test lists to manually include/exclude the stress tests is surely a 
tedious job, but it is something that would be done once for every new test.
Trying to find the right set of tests to run on a change is a problem that every 
developer is facing more or less on a daily basis. I think it is more important 
to make this the easy task.

/Jesper


>
>
> 3)
>
> please add:
>  * @key gc stress
>
> you don't need:
>  * @build gc.logging.TestUnifiedLoggingSwitchStress
>
>
> 4) Other comments are rather minor recommendations:
>
>     private final int filesCount;
>     private final String fileNamePrefix;
> -->
>     private final int logCount;  // how many various log files will be used
>     private final String logFilePrefix;  // name of log file will be
> logFilePrefix + index
>
>
>
>     System.out.format("Diagnostic command vmLog with arguments %s,%s returned
> not empty output %s\n",
> -->
>     System.out.format("WARNING: Diagnostic command vmLog with arguments %s,%s
> returned not empty output %s\n",
>
>     LinkedList<Thread> threads = new LinkedList<>();
> -->
>     List<Thread> threads = new LinkedList<>();
>
>     Calendar calendar = Calendar.getInstance();
>     long startTime = calendar.getTimeInMillis();
> -->
>     long startTime = System.currentTimeMillis()
>
>
>     while (Calendar.getInstance().getTimeInMillis() - startTime < duration) {
>         Thread.yield();
>     }
> -->
>     while (System.currentTimeMillis() - startTime < duration) {
>         Thread.sleep(1000);
>     }
>
>
>
> Thanks,
> Dima
>
> On 06.05.2016 14:10, Kirill Zhaldybin wrote:
>> Dear all,
>>
>> Could you please review this fix for 8150865?
>>
>> The test creates two types of threads: the first type stresses gc and the
>> second type switches gc unified loggingl level/file output.
>> The test is a stress test and it is expected that it finishes normally (no oom
>> or crash).
>>
>> WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8150865/webrev.00/
>> CR: https://bugs.openjdk.java.net/browse/JDK-8150865
>>
>> Thank you.
>>
>> Regards, Kirill
>