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

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Tue May 10 14:26:15 UTC 2016


Dmitry,

Thank you for reviewing the fix!

Here are a new WebRev:
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8150865/webrev.02/

I hope I addressed your comments in it.

Could you please let me know your opinion?

Regards, Kirill


On 10.05.2016 09:12, Dmitry Fazunenko wrote:
> 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
>
>
> 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
>




More information about the hotspot-gc-dev mailing list