RFR: 8177968: Add GC stress test TestGCLocker

[Leonid Mesnik]

Hi

Sorry for late response.

Could you please add tag ' @key stress'  to the tests. It helps to 
exclude them using keywords when only fast tests are going to be run.
I am not sure if it is used right now in the test execution for gc 
tests. However it is a good practice and all other gc stress tests are 
marked.

Leonid


On 18.04.2017 12:22, Erik Helin wrote:
> On 04/12/2017 08:05 AM, Per Liden wrote:
>> Hi Erik,
>>
>> (Re-sending, as Thunderbird crashed on me and it seems that my first
>> reply never made it to the list)
>
> Hey Per,
>
> thanks for reviewing! Please see new version at
> - inc: http://cr.openjdk.java.net/~ehelin/8177968/00-01/
> - full: http://cr.openjdk.java.net/~ehelin/8177968/01/
>
> and comments inline:
>
>> On 2017-04-07 17:48, Erik Helin wrote:
>>> Hi all,
>>>
>>> this patch adds the stress test "TestGCLocker". The test repeatedly
>>> calls GCLocker::lock_critical/unlock_critical (via the JNI functions
>>> GetPrimitiveArrayCritical/ReleasePrimitiveArrayCritical) while
>>> concurrently filling up the old gen. Thea idea is to stress the 
>>> GCLocker
>>> implementation by quickly entering/leaving critical JNI sections while
>>> simultaneously allocating objects to fill up the heap in order to
>>> provoke a GC.
>>>
>>> Enhancement:
>>> https://bugs.openjdk.java.net/browse/JDK-8177968
>>>
>>> Patch:
>>> http://cr.openjdk.java.net/~ehelin/8177968/00/
>>
>> I did some test runs and it seems the test only provokes the wanted
>> situation in ~20% of the GCs (on my machine at least). Being a stress
>> test, I'd like to propose that we remove the sleep() calls to have it
>> provoke the situation in ~100% of the GCs.
>
> Thanks for taking the code for a spin!
>
>> --- a/test/gc/stress/gclocker/TestGCLocker.java
>> +++ b/test/gc/stress/gclocker/TestGCLocker.java
>> @@ -162,7 +153,6 @@
>>
>>          while (!shouldExit()) {
>>              load();
>> -            ThreadUtils.sleep(100);
>>          }
>>      }
>>  }
>> @@ -175,7 +165,6 @@
>>          byte[] array = new byte[1024 * 1024];
>>          while (!shouldExit()) {
>>              fillWithRandomValues(array);
>> -            ThreadUtils.sleep(10);
>>          }
>>      }
>>  }
>
> Great suggestion, will update the patch.
>
>> Also, the filler function only writes to the first byte, which looks
>> like a bug. Simple fix:
>>
>> --- a/test/gc/stress/gclocker/libTestGCLocker.c
>> +++ b/test/gc/stress/gclocker/libTestGCLocker.c
>> @@ -29,7 +29,7 @@
>>    jbyte* p = (*env)->GetPrimitiveArrayCritical(env, arr, NULL);
>>    jsize i;
>>    for (i = 0; i < size; i++) {
>> -      *p = i % 128;
>> +    p[i] = i % 128;
>>    }
>>    (*env)->ReleasePrimitiveArrayCritical(env, arr, p, 0);
>>  }
>
> Heh, thanks for noticing this :) Reminds me of:
> http://dilbert.com/strip/2001-10-25
>
>> Other than that, looks good!
>
> Thanks!
> Erik
>
>> cheers,
>> Per
>>
>>> Testing:
>>> - JPRT (to ensure libTestGCLocker.c compiles on all platforms)
>>> - make run-test TEST=hotspot/test/gc/stress/gclocker
>>>
>>> Thanks,
>>> Erik