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

[Tobias Hartmann]

Hi Vladimir,

thanks for the review.

On 29.08.2014 01:13, Vladimir Kozlov wrote:
> For the record, SegmentedCodeCache is enabled by default when 
> TieredCompilation is enabled and ReservedCodeCacheSize >= 240 MB. 
> Otherwise it is false by default.

Exactly.

> arguments.cpp - in set_tiered_flags() swap SegmentedCodeCache setting 
> and segments size adjustment - do adjustment only if 
> SegmentedCodeCache is enabled. 

Done.

> Also I think each flag should be checked and adjusted separately. You 
> may bail out (vm_exit_during_initialization) if sizes do not add up. 

I think we should only increase the sizes if they are all default. 
Otherwise we would for example fail if the user sets the 
NonMethodCodeHeapSize and the ProfiledCodeHeapSize because the 
NonProfiledCodeHeap size is multiplied by 5. What do you think?

> And use

I think the rest of this sentence is missing :)

> Align second line:
>
> 2461   } else if ((!FLAG_IS_DEFAULT(NonMethodCodeHeapSize) || 
> !FLAG_IS_DEFAULT(ProfiledCodeHeapSize) || 
> !FLAG_IS_DEFAULT(NonProfiledCodeHeapSize))
> 2462       && (NonMethodCodeHeapSize + NonProfiledCodeHeapSize + 
> ProfiledCodeHeapSize) != ReservedCodeCacheSize) {

Done.

> codeCache.cpp - in initialize_heaps() add new methods in C1 and C2 to 
> return buffer_size they need. Add assert(SegmentedCodeCache) to this 
> method to show that we call it only in such case. 

Done.

> You do adjustment only when all flags are default. But you still need 
> to check that you have space in NonMethodCodeHeap for scratch buffers.

I added a the following check:

   // Make sure we have enough space for the code buffers
   if (NonMethodCodeHeapSize < code_buffers_size) {
     vm_exit_during_initialization("Not enough space for code buffers in 
CodeCache");
   }

> codeCache.hpp - comment alignment:
> +     // Creates a new heap with the given name and size, containing 
> CodeBlobs of the given type
> !   static void add_heap(ReservedSpace rs, const char* name, size_t 
> size_initial, int code_blob_type);

Done.

> nmethod.cpp - in new() can we mark nmethod allocation critical only 
> when SegmentedCodeCache is enabled?

Yes, that's what we do with:

809   bool is_critical = SegmentedCodeCache;

Or what are you referring to?

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

Thanks,
Tobias

> Thanks,
> Vladimir
>
> On 8/28/14 5:09 AM, 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