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

[Vladimir Kozlov]

On 8/29/14 7:10 AM, Tobias Hartmann wrote:
> 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?

But ReservedCodeCacheSize is scaled anyway and you will get sum of sizes != whole size. We need to do something.

BTW the error message for next check should print all sizes, user may not know the default value of some which he did 
not specified on command line.

(NonMethodCodeHeapSize + NonProfiledCodeHeapSize + ProfiledCodeHeapSize) != ReservedCodeCacheSize)

>
>> And use
>
> I think the rest of this sentence is missing :)

And use FLAG_SET_ERGO() when you scale. :)

>
>> 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");
>    }

I think, you need to take into account min_code_cache_size as in arguments.cpp:
  uint min_code_cache_size = (CodeCacheMinimumUseSpace DEBUG_ONLY(* 3)) + CodeCacheMinimumFreeSpace;

if (NonMethodCodeHeapSize < (min_code_cache_size+code_buffers_size)) {

I would be nice if this code in initialize_heaps() could be moved called during arguments parsing if we could get number 
of compiler threads there. But I understand that we can't do that until compilation policy is set :(

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

Somehow I missed that SegmentedCodeCache is used already. It is fine then.

Thanks,
Vladimir

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