[11] RFR(S) 8207069: [AOT] we should check that VM uses the same GC as one used for AOT library generation.

[Vladimir Kozlov]

On 7/18/18 2:25 PM, Christian Thalinger wrote:
> 
> 
>> On Jul 18, 2018, at 3:08 PM, Vladimir Kozlov 
>> <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>
>> On 7/18/18 6:15 AM, Christian Thalinger wrote:
>>>> On Jul 18, 2018, at 12:14 AM, Vladimir Kozlov 
>>>> <vladimir.kozlov at oracle.com 
>>>> <mailto:vladimir.kozlov at oracle.com><mailto:vladimir.kozlov at oracle.com>> 
>>>> wrote:
>>>>
>>>> On 7/17/18 5:17 PM, Christian Thalinger wrote:
>>>>>> On Jul 12, 2018, at 5:28 PM, Vladimir Kozlov 
>>>>>> <vladimir.kozlov at oracle.com 
>>>>>> <mailto:vladimir.kozlov at oracle.com><mailto:vladimir.kozlov at oracle.com>> 
>>>>>> wrote:
>>>>>>
>>>>>> Including GC group since I added new method to GCConfig.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~kvn/8207069/webrev.00/ 
>>>>>> <http://cr.openjdk.java.net/%7Ekvn/8207069/webrev.00/><http://cr.openjdk.java.net/%7Ekvn/8207069/webrev.00/>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8207069
>>>>>>
>>>>>> Recent Graal's changes [1] added list of GC [2] which matches 
>>>>>> Hotspot GC list [3].
>>>>>> I used that to fix this issue by storing enum value from Graal in 
>>>>>> AOT config header and compare it with selected GC when AOT library 
>>>>>> is loaded into Hotspot.
>>>>> The fix is correct but too strict.  For example, Serial and 
>>>>> Parallel GC can use the same AOT library. CMS too.
>>>>
>>>> Do you have other suggestions how to check compatibility?
>>> I think the best way would be to use BarrierSet::Name:
>>> // Do something for each concrete barrier set part of the build.
>>> #define FOR_EACH_CONCRETE_BARRIER_SET_DO(f)          \
>>> f(CardTableBarrierSet)                             \
>>> EPSILONGC_ONLY(f(EpsilonBarrierSet))               \
>>> G1GC_ONLY(f(G1BarrierSet))                         \
>>> ZGC_ONLY(f(ZBarrierSet))
>>
>> Thank you, Chris, for suggestion.
>>
>> To record barrier set in AOT library would require a lot more complex 
>> changes (JVMCI) not suitable for JDK 11.  Currently Graal checks only 
>> GC flags. To get information about barrier set it needs to access 
>> Hotspot's data.
> 
> Yeah, that’s a problem.
> 
>>
>> The only simple way to relax the check is to get BarrierSet::Name 
>> value based on CollectedHeap::Name and compare them in aot library 
>> config check code. But I can't find a functionality in GC code to do 
>> that. I asked GC group.

I got answer from GC that they don't have such mapping and don't think 
it is needed.

>>
>> Note, we never intended to support mixed GCs with the same type of 
>> barriers. It was accidental and I am not comfortable to support such 
>> "feature”.
> 
> You mean that two different GCs use the same barrier set?  Yes, I agree, 
> it would be better if each had their own.
> 
> Do you want to push your current patch?

Yes. I am waiting PR review from Labs since AOT code (jaotc) is now there.

Thanks,
Vladimir

> 
>>
>> Vladimir
>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>>
>>>>>> I verified the fix with all GCs combination when compiling AOT lib 
>>>>>> and using it. I also ran our hs-tier1-3 testing which includes AOT 
>>>>>> and Graal tests.
>>>>>>
>>>>>> These changes are for JDK 11 so I don't need to go through Graal 
>>>>>> PR now but I would need to do that for JDK 12 to make changes in 
>>>>>> AOT code.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8205824
>>>>>>  "[GR-10514] Use whitelist for GCs supported by Graal"
>>>>>> [2] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk11/file/bf686c47c109/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/HotSpotGraalRuntime.java#l137
>>>>>> [3] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk11/file/bf686c47c109/src/hotspot/share/gc/shared/collectedHeap.hpp#l173
>