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

Fri Mar 29 07:36:40 UTC 2019

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