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

[Vladimir Kozlov]

Looks good. You need second review.

And, please, add a WB test which verifies a reaction on flags settings 
similar to what test/compiler/codecache/CheckUpperLimit.java does. Both 
positive (SegmentedCodeCache is enabled) and negative (sizes does not 
match or ReservedCodeCacheSize is small) and etc.

Thanks,
Vladimir

On 9/3/14 4:10 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> thanks for the review.
>
> On 29.08.2014 19:33, Vladimir Kozlov wrote:
>> 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.
>
> I agree. I changed it as you suggested first: The code heap sizes are
> scaled individually and we bail out if the sizes are not consistent with
> ReservedCodeCacheSize.
>
>> 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)
>
> The error message now prints the sizes in brackets.
>
>>>> And use
>>>
>>> I think the rest of this sentence is missing :)
>>
>> And use FLAG_SET_ERGO() when you scale. :)
>
> Done. I also changed the implementation of CodeCache::initialize_heaps()
> accordingly.
>
>>>> 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)) {
>
> True, I changed it.
>
>> 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 :(
>
> Yes, this is not possible because we need to know the number of C1/C2
> compiler threads.
>
> New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.05/
>
> Thanks,
> Tobias
>
>>
>>>
>>>> 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
>>>
>