RFR: 8177968: Add GC stress test TestGCLocker

[Erik Helin]

On 04/18/2017 11:56 AM, Per Liden wrote:
> Hi Erik,
>
> On 2017-04-18 11: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/
>
> Thanks for fixing. Looks good!

Thanks!
Erik

> /Per
>
>>
>> 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