RFR (S): 8214086: [TESTBUG] Fix subgraph test cases in ReplaceCriticalClasses.java

[Jiangli Zhou]

Thanks!

Jiangli


On 11/21/18 5:34 PM, Ioi Lam wrote:
> Looks good. Thanks!
>
> - Ioi
>
> On 11/21/18 4:34 PM, Jiangli Zhou wrote:
>> Here is the updated webrev with a new test case suggested by Ioi:
>>
>> http://cr.openjdk.java.net/~jiangli/8214086/webrev.01/test/hotspot/jtreg/runtime/SharedArchiveFile/serviceability/ReplaceCriticalClasses.java.frames.html 
>>
>>
>> Full webrev: http://cr.openjdk.java.net/~jiangli/8214086/webrev.01/
>>
>> Thanks,
>> Jiangli
>>
>> On 11/21/18 3:23 PM, Jiangli Zhou wrote:
>>> Hi Ioi,
>>>
>>> On 11/21/18 3:08 PM, Ioi Lam wrote:
>>>> Hi Jiangli,
>>>>
>>>> For this test
>>>>
>>>> final String subgraphInit = "initialize_from_archived_subgraph " + 
>>>> subgraphKlass;
>>>> ...
>>>> if (checkSubgraph) {
>>>>      out.shouldNotContain(subgraphInit);
>>>> }
>>>>
>>>> I think you should also test for the reverse condition -- if 
>>>> java/lang/Long has not been redefined, you should see the 
>>>> subgraphInit in the output.
>>>>
>>>> The reason for this is shouldNotContain() is very lenient. If you 
>>>> made a mistake in formatting subgraphInit, shouldNotContain will 
>>>> not complain, and you will end up having a bad test that doesn't 
>>>> test what you're looking for.
>>>
>>> I actually have the reverse check initially, but decided to remove 
>>> it since we have other tests that explicitly check for archived Long 
>>> cache and system module subgraphs. I can add one test case for the 
>>> Long cache (for future proof in case the logging output is changed).
>>>
>>> Thanks,
>>> Jiangli
>>>
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>>
>>>>
>>>> On 11/21/18 2:17 PM, Jiangli Zhou wrote:
>>>>> Please review the following test fix in 
>>>>> ReplaceCriticalClasses.java. I re-implemented the subgraph check 
>>>>> to verify a subgraph is not used at runtime if one of it's klass 
>>>>> (who's object is contained within the archived subgraph) is 
>>>>> changed by the JVMTI agent.
>>>>>
>>>>> I added a test case for the newly archived Long cache subgraph. 
>>>>> Since both ResolvedModule and Long classes are loaded during the 
>>>>> primordial phase, '-early -notshared' are set in those test cases.
>>>>>
>>>>> As part of the change, I fixed 
>>>>> HeapShared::initialize_from_archived_subgraph to only do the 
>>>>> logging output after sbugraph(s) is successfully installed in the 
>>>>> given Klass.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~jiangli/8214086/webrev.00
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8214086
>>>>>
>>>>> Tested ReplaceCriticalClasses.java locally on linux-x64. Will run 
>>>>> tier1-tier2.
>>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>
>>