[9] RFR (L): 8015774: Add support for multiple code heaps
Igor Veresov
igor.veresov at oracle.com
Tue Sep 16 08:23:54 UTC 2014
On Sep 16, 2014, at 12:10 AM, Tobias Hartmann <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> wrote:
>>
>>> Hi,
>>>
>>> could I get another review for this?
>>>
>>> Latest webrev is: http://cr.openjdk.java.net/~thartmann/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/
>>>> JDK-Test fix: http://cr.openjdk.java.net/~thartmann/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