RFR (S) 8236901: 8232759 missed a test case

[David Holmes]

Hi Coleen,

On 14/01/2020 1:04 pm, coleen.phillimore at oracle.com wrote:
> 
> 
> On 1/13/20 7:36 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 14/01/2020 7:08 am, coleen.phillimore at oracle.com wrote:
>>> Summary: Use jcmd GC.class-histogram because it also works for 
>>> verifying that the classes are loaded.
>>
>> I'm looking at GC.class_stats and GC.class_histogram output and the 
>> actual test and I'm not sure the test was even working the way it was 
>> expected before. If I run the test using GC.class_stats I see:
>>
>> ----------System.out:(20/1962)----------
>> sizeof(DefineClass.class) == 8905
>> class test.DefineClass
>> WARNING: Ran out of C-heap; undercounted 342 total instances in data 
>> below
>>  1447    46         0        544           0    6752          12 
>> 1351      4048    3616    8088   11704 test.DefineClass
>>  1448    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1449    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1450    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1451    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1452    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1453    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1454    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1455    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1456    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>>  1457    46         0        544           0    4072          12 
>> 1351      4048    3536    5488    9024 test.DefineClass
>> Should have 2 DefineClass instances and we have: 11
> 
> I think the test wants *at least* 2 DefineClasses in these places, where 
> it passes false to getClassStats.

Not according to this:

      // We expect to have two instances of DefineClass here: the 
initial version in which we are
      // executing and another version which was loaded into our own 
classloader 'MyClassLoader'.
      // All the subsequent attempts to reload DefineClass into our 
'MyClassLoader' should have failed.
      printClassStats(2, false);

However this is a complex test and some of the other variants do expect 
11 instances, so this may just be poorly documented in the test.

>>
>> then
>>
>> System.gc()
>> WARNING: Ran out of C-heap; undercounted 342 total instances in data 
>> below
>>  1598    47         0        544           0    6752          12 
>> 1351      4048    3616    8088    11704 test.DefineClass
>>  1599    47         0        544           0    4072          12 
>> 1351      4048    3536    5488     9024 test.DefineClass
>> Should have 2 DefineClass instances and we have: 2
>>
>> so the test "passes". But if I then change the test to use 
>> GC.class_histogram as suggested it fails!
>>
>>
>> ----------System.out:(7/321)----------
>> sizeof(DefineClass.class) == 8909
>> class test.DefineClass
>> WARNING: Ran out of C-heap; undercounted 342 total instances in data 
>> below
>> Should have 2 DefineClass instances and we have: 0
>> System.gc()
>> WARNING: Ran out of C-heap; undercounted 342 total instances in data 
>> below
>> Should have 2 DefineClass instances and we have: 0
>>
> 
> I don't know why it fails for you.  How did you run it?  Did you test my 
> patch?

I applied your change to my test locally. But I realized why it is 
probably failing is the:

WARNING: Ran out of C-heap; undercounted 342 total instances

as the class we are looking for may be one of the instances that 
couldn't be counted and presumably reported. Do you see this C-Heap 
warning when you run the test?

In conclusion I think your fix for the test is fine, but the test itself 
and the underlying jcmd stuff has some potential issues.

Thanks,
David
-----

> Coleen
> 
>> This is on a JDK where GC.class_stats still exists.
>>
>> David
>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2020/8236901.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8236901
>>>
>>> Ran tier1 which runs the test on all Oracle platforms.
>>>
>>> Thanks,
>>> Coleen
>