[9] RFR (L): 8015774: Add support for multiple code heaps

[Tobias Hartmann]

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.

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

New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.08/

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