8178095: Add GC stress test TestSystemGC

Erik Helin erik.helin at oracle.com
Wed Apr 5 12:40:40 UTC 2017


Hey Dima,

thanks for having a look! Please see a new version at:
- inc: http://cr.openjdk.java.net/~ehelin/8177967/00-01/
- full: http://cr.openjdk.java.net/~ehelin/8177967/01/

On 04/05/2017 10:43 AM, Dmitry Fazunenko wrote:
> Hi Erik,
>
> Overall the fix looks good to me.
> Some minor questions/comments:
>
> 1. Have you tested your code with -XX:-UseCompressedOpps ?
>     (you create a lot of objects and limit the heap by 512m)

Well, it is meant to be run according to the @run tags. If a developer 
runs with different flags, they are on their own :) But yeah, I took it 
for a spin and it works (at least on my machine).

> 2. Would you mind renaming TestSystemGC to SystemGCTest or similar. Just
> to distinguish jtreg tests from libraries.

Hmm, all the other stress tests already use the Test*.java convention, 
like TestGCBasher and TestGCOld. Some of the other tests do prefix with 
TestStress*.java, but not all.

I would prefer to keep the name the file TestSystemGC.java, to follow 
the existing convention. Since the 'stress' directory should already 
indicate that this is a stress test, I would prefer to not prefix with 
'TestStress'.

> 3. Adding a couple of sentences explaining the idea of TestSystemGC
> would help.

I tried to do that as part of the @summary tag in each jtreg test :) I 
added small comment at the top of TestSystemGC as well.

> 4. You run the same method twice:
>
>     public static void main(String[] args) {
>         populateLongLived();
>         runAllPhases();
>         runAllPhases();
>     }
>
> Is it intentionally? If yes, would you add a small comment to the second
> run (to avoid possible confusion)

Yep, intentionally, I added a comment.

> 5. Optional: you can move the sleep() method from ThreadUtils  to
> TestSystemGC (to reduce the number of classes)

I think I keep it in ThreadUtils for now, thanks,

Erik

> Thanks,
> Dima
>
>
> On 05.04.2017 10:14, Erik Helin wrote:
>> Hi all,
>>
>> this patch adds a small stress test called TestSystemGC. The test
>> stresses a full GC implementation by allocating objects of different
>> life times while concurrently calling System.gc() repeatedly.
>>
>> Enhancement:
>> https://bugs.openjdk.java.net/browse/JDK-8178095
>>
>> Patch:
>> http://cr.openjdk.java.net/~ehelin/8178095/00/
>>
>> Testing:
>> - make run-test TEST=hotspot/test/gc/stress/systemgc
>>
>> Thanks,
>> Erik
>


More information about the hotspot-gc-dev mailing list