[13] RFR(L) 8220623: [JVMCI] Update JVMCI to support JVMCI based Compiler compiled into shared library

Mon Apr 1 07:34:06 UTC 2019

On 2019-03-29 17:55, Vladimir Kozlov wrote:
> Stefan,
> 
> Do you have a test (and flags) which can allow me to measure effect of 
> this code on G1 remark pause?


-Xlog:gc prints the remark times:
[4,296s][info][gc       ] GC(89) Pause Remark 4M->4M(28M) 36,412ms

StefanK

> 
> Thanks,
> Vladimir
> 
> On 3/29/19 12:36 AM, Stefan Karlsson wrote:
>> On 2019-03-29 03:07, Vladimir Kozlov wrote:
>>> Hi Stefan,
>>>
>>> I collected some data on MetadataHandleBlock.
>>>
>>> First, do_unloading() code is executed only when 
>>> class_unloading_occurred is 'true' - it is rare case. It should not 
>>> affect normal G1 remark pause.
>>
>> It's only rare for applications that don't do dynamic class loading 
>> and unloading. The applications that do, will be affected.
>>
>>>
>>> Second, I run a test with -Xcomp. I got about 10,000 compilations by 
>>> Graal and next data at the end of execution:
>>>
>>> max_blocks = 232
>>> max_handles_per_block = 32 (since handles array has 32 elements)
>>> max_total_alive_values = 4631
>>
>> OK. Thanks for the info.
>>
>> StefanK
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 3/28/19 2:44 PM, Vladimir Kozlov wrote:
>>>> Thank you, Stefan
>>>>
>>>> On 3/28/19 12:54 PM, Stefan Karlsson wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> I started to check the GC code.
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> I see that you've added guarded includes in the middle of the 
>>>>> include list:
>>>>>    #include "gc/shared/strongRootsScope.hpp"
>>>>>    #include "gc/shared/weakProcessor.hpp"
>>>>> + #if INCLUDE_JVMCI
>>>>> + #include "jvmci/jvmci.hpp"
>>>>> + #endif
>>>>>    #include "oops/instanceRefKlass.hpp"
>>>>>    #include "oops/oop.inline.hpp"
>>>>>
>>>>> The style we use is to put these conditional includes at the end of 
>>>>> the include lists.
>>>>
>>>> okay
>>>>
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> Could you also change the following:
>>>>>
>>>>> + #if INCLUDE_JVMCI
>>>>> +     // Clean JVMCI metadata handles.
>>>>> +     JVMCI::do_unloading(is_alive_closure(), purged_class);
>>>>> + #endif
>>>>>
>>>>> to:
>>>>> +     // Clean JVMCI metadata handles.
>>>>> +     JVMCI_ONLY(JVMCI::do_unloading(is_alive_closure(), 
>>>>> purged_class);)
>>>>>
>>>>> to get rid of some of the line noise in the GC files.
>>>>
>>>> okay
>>>>
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> In the future we will need version of JVMCI::do_unloading that 
>>>>> supports concurrent cleaning for ZGC.
>>>>
>>>> Yes, we need to support concurrent cleaning in a future.
>>>>
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> What's the performance impact for G1 remark pause with this serial 
>>>>> walk over the MetadataHandleBlock?
>>>>>
>>>>> 3275 void G1CollectedHeap::complete_cleaning(BoolObjectClosure* 
>>>>> is_alive,
>>>>> 3276                                         bool 
>>>>> class_unloading_occurred) {
>>>>> 3277   uint num_workers = workers()->active_workers();
>>>>> 3278   ParallelCleaningTask unlink_task(is_alive, num_workers, 
>>>>> class_unloading_occurred, false);
>>>>> 3279   workers()->run_task(&unlink_task);
>>>>> 3280 #if INCLUDE_JVMCI
>>>>> 3281   // No parallel processing of JVMCI metadata handles for now.
>>>>> 3282   JVMCI::do_unloading(is_alive, class_unloading_occurred);
>>>>> 3283 #endif
>>>>> 3284 }
>>>>
>>>> There should not be impact if Graal is not used. Only cost of call 
>>>> (which most likely is inlined in product VM) and check:
>>>>
>>>> http://hg.openjdk.java.net/metropolis/dev/file/530fc1427d02/src/hotspot/share/jvmci/jvmciRuntime.cpp#l1237 
>>>>
>>>>
>>>> If Graal is used it should not have big impact since these metadata 
>>>> has regular pattern (32 handles per array and array per 
>>>> MetadataHandleBlock block which are linked in list) and not large.
>>>> If there will be noticeable impact - we will work on it as you 
>>>> suggested by using ParallelCleaningTask.
>>>>
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> Did you consider adding it as a task for one of the worker threads 
>>>>> to execute in ParallelCleaningTask?
>>>>>
>>>>> See how other tasks are claimed by one worker:
>>>>> void KlassCleaningTask::work() {
>>>>>    ResourceMark rm;
>>>>>
>>>>>    // One worker will clean the subklass/sibling klass tree.
>>>>>    if (claim_clean_klass_tree_task()) {
>>>>>      Klass::clean_subklass_tree();
>>>>>    }
>>>>
>>>> These changes were ported from JDK8u based changes in graal-jvmci-8 
>>>> and there are no ParallelCleaningTask in JDK8.
>>>>
>>>> Your suggestion is interesting and I agree that we should 
>>>> investigate it.
>>>>
>>>>>
>>>>> ======================================================================== 
>>>>>
>>>>> In MetadataHandleBlock::do_unloading:
>>>>>
>>>>> +        if (klass->class_loader_data()->is_unloading()) {
>>>>> +          // This needs to be marked so that it's no longer scanned
>>>>> +          // but can't be put on the free list yet. The
>>>>> +          // ReferenceCleaner will set this to NULL and
>>>>> +          // put it on the free list.
>>>>>
>>>>> I couldn't find the ReferenceCleaner in the patch or in the source. 
>>>>> Where can I find this code?
>>>>
>>>> I think it is typo (I will fix it) - it references new HandleCleaner 
>>>> class:
>>>>
>>>> http://cr.openjdk.java.net/~kvn/8220623/webrev.03/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HandleCleaner.java.html 
>>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>
>>>>> Thanks,
>>>>> StefanK
>>>>>
>>>>> On 2019-03-28 20:15, Vladimir Kozlov wrote:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220623
>>>>>> http://cr.openjdk.java.net/~kvn/8220623/webrev.03/
>>>>>>
>>>>>> Update JVMCI to support pre-compiled as shared library Graal.
>>>>>> Using aoted Graal can offers benefits including:
>>>>>>  - fast startup
>>>>>>  - compile time similar to native JIt compilers (C2)
>>>>>>  - memory usage disjoint from the application Java heap
>>>>>>  - no profile pollution of JDK code used by the application
>>>>>>
>>>>>> This is JDK13 port of JVMCI changes done in graal-jvmci-8 [1] up 
>>>>>> to date.
>>>>>> Changes were collected in Metropolis repo [2] and tested there.
>>>>>>
>>>>>> Changes we reviewed by Oracle Labs (authors of JVMCI and Graal) 
>>>>>> and our compiler group.
>>>>>> Changes in shared code are guarded by #if INCLUDE_JVMCI and JVMCI 
>>>>>> flags.
>>>>>>
>>>>>> I ran tier1-tier8 (which includes HotSpot and JDK tests) and it 
>>>>>> was clean. In this set Graal was tested only in tier3.
>>>>>>
>>>>>> And I ran all hs-tier3-graal .. hs-tier8-graal Graal tests 
>>>>>> available in our system. Several issue were found which were 
>>>>>> present before these changes.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> [1] 
>>>>>> https://github.com/graalvm/graal-jvmci-8/commit/49ff2045fb603e35516a3a427d8023c00e1607af 
>>>>>>
>>>>>> [2] http://hg.openjdk.java.net/metropolis/dev/
>>>>>