[9] RFR (L): 8015774: Add support for multiple code heaps
Tobias Hartmann
tobias.hartmann at oracle.com
Tue Sep 16 08:31:44 UTC 2014
Igor, thanks for the review.
Best,
Tobias
On 16.09.2014 10:23, Igor Veresov wrote:
>
> On Sep 16, 2014, at 12:10 AM, Tobias Hartmann
> <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>
>> Hi Igor,
>>
>> thanks for the review.
>>
>> On 16.09.2014 01:18, Igor Veresov wrote:
>>> A little thing that worried me.. c2i go to i2c adapters go to the
>>> NonMethod space (via BufferBlob::new), which is fixed and not
>>> scaled. However MH intrinsics (and native adapters) go to
>>> MethodNonProfile space. Since the number of c2i and i2c are
>>> signature polymorphic (and each MH intrinsic has them) perhaps they
>>> should go to the MethodNonProfiled space as well? AdapterBlob would
>>> have to have a different new operator and the OOM handing code in
>>> AdapterHandleLibrary::get_adapter() will have to adjusted.
>>
>> If the NonMethod segment is full we allocate new BufferBlobs in the
>> MethodNonProfiled segment. See 'CodeCache::allocate':
>>
>> if (SegmentedCodeCache && (code_blob_type ==
>> CodeBlobType::NonMethod)) {
>> // Fallback solution: Store non-method code in the
>> non-profiled code heap
>> return allocate(size, CodeBlobType::MethodNonProfiled,
>> is_critical);
>> }
>>
>> In the case of c2i and i2c adapters we first try to allocate them in
>> the NonMethod segment and if this fails "fall back" to the
>> MethodNonProfiled segment. The main advantage of this solution is
>> that we avoid having non-method code in the method segments as long
>> as possible.
>
> Ok, makes sense.
>
>>
>>> Nits:
>>>
>>> codeCache.cpp:
>>> 141 // Initialize array of CodeHeaps
>>> 142 GrowableArray<CodeHeap*>* CodeCache::_heaps = new(ResourceObj::C_HEAP, mtCode) GrowableArray<CodeHeap*> (3, true);
>>> Perhaps 3 should be a named constant. May be you can put it in the
>>> enum with segment types you have in CodeBlobType ?
>>
>> Yes, I replaced 3 by the existing constant 'CodeBlobType::All'.
>>
>>> advancedThresholdPolicy.cpp:
>>> 213 // Increase C1 compile threshold when the code cache is filled more
>>> 214 // than specified by IncreaseFirstTierCompileThresholdAt percentage.
>>> 215 // The main intention is to keep enough free space for C2 compiled code
>>> 216 // to achieve peak performance if the code cache is under stress.
>>> 217 if ((TieredStopAtLevel == CompLevel_full_optimization) && (level != CompLevel_full_optimization)) {
>>> 218 double current_reverse_free_ratio = CodeCache::reverse_free_ratio(CodeCache::get_code_blob_type(level));
>>> 219 if (current_reverse_free_ratio > _increase_threshold_at_ratio) {
>>> 220 k *= exp(current_reverse_free_ratio - _increase_threshold_at_ratio);
>>> 221 }
>>> 222 }
>>> Do you think it still makes sense to do that with segmented code
>>> cache? C1 methods are not really going to take space from C2
>>> methods, right? Perhaps it should be predicated off for segmented
>>> code cache?
>>
>> Thanks for catching this. Yes, C1 methods do not take space from C2
>> methods. I tried to disable that part some time ago during
>> development and it caused problems with too much C1 code being
>> generated. The sweeper did not remove methods fast enough and the
>> profiled code heap filled up.
>>
>> I would prefer to re-investigate the removal of these lines after the
>> initial integration of the segmented code cache. I planned to file an
>> RFE for "per segment sweeping", i.e., sweeping the profiled segment
>> more often. Also, Albert's fix for JDK-8046809 [1] will probably
>> affect the behaviour of the sweeper.
>>
>> What do you think?
>
> Having a second look, I think this code is fine. It’s ok to increase
> the threshold for profiled code if there’s too much space pressure.
> Even if we don’t compile it with C1, we’ll start profiling in the
> interpreter, which is a reasonable behavior. However we might want to
> adjust the profile code cache size if the behavior you describe
> happens with real apps because that’s suboptimal.
>
>>
>> New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.08/
>
>
> Looks fine.
>
> Thanks,
> igor
>
>
>>
>> Thanks,
>> Tobias
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8046809
>>
>>>
>>> Otherwise looks good.
>>>
>>> igor
>>>
>>>
>>> On Sep 5, 2014, at 1:53 AM, Tobias Hartmann
>>> <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>>>
>>>> Hi,
>>>>
>>>> could I get another review for this?
>>>>
>>>> Latest webrev is:
>>>> http://cr.openjdk.java.net/~thartmann/8015774/webrev.07/
>>>> <http://cr.openjdk.java.net/%7Ethartmann/8015774/webrev.07/>
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>> On 28.08.2014 14:09, Tobias Hartmann wrote:
>>>>> Hi,
>>>>>
>>>>> the segmented code cache JEP is now targeted. Please review the
>>>>> final implementation before integration. The previous RFR,
>>>>> including a short description, can be found here [1].
>>>>>
>>>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8043304
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8015774
>>>>> Implementation:
>>>>> http://cr.openjdk.java.net/~thartmann/8015774/webrev.03/
>>>>> <http://cr.openjdk.java.net/%7Ethartmann/8015774/webrev.03/>
>>>>> JDK-Test fix:
>>>>> http://cr.openjdk.java.net/~thartmann/8015774_jdk_test/webrev.00/
>>>>> <http://cr.openjdk.java.net/%7Ethartmann/8015774_jdk_test/webrev.00/>
>>>>>
>>>>> Changes since the last review:
>>>>> - Merged with other changes (for example, G1 class unloading
>>>>> changes [2])
>>>>> - Fixed some minor bugs that showed up during testing
>>>>> - Refactoring of 'NMethodIterator' and CodeCache implementation
>>>>> - Non-method CodeHeap size increased to 5 MB
>>>>> - Fallback solution: Store non-method code in the non-profiled
>>>>> code heap if there is not enough space in the non-method code heap
>>>>> (see 'CodeCache::allocate')
>>>>>
>>>>> Additional testing:
>>>>> - BigApps (Weblogic, Dacapo, runThese, Kitchensink)
>>>>> - Compiler and GC nightlies
>>>>> - jtreg tests
>>>>> - VM (NSK) Testbase
>>>>> - More performance testing (results attached to the bug)
>>>>>
>>>>> Thanks,
>>>>> Tobias
>>>>>
>>>>> [1]
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2014-April/014098.html
>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8049421
>>>>
>>>
>>
>
More information about the hotspot-dev
mailing list