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

[Christian Thalinger]

> On Jul 18, 2018, at 5:51 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> 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><mailto: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 <mailto: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 <mailto: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/~kvn/8207069/webrev.00/><http://cr.openjdk.java.net/%7Ekvn/8207069/webrev.00/ <http://cr.openjdk.java.net/%7Ekvn/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.

Sounds good.  You can use me as a Reviewer, if needed.

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180718/c23ac53c/attachment.html>