[9] RFR (L): 8015774: Add support for multiple code heaps
Tobias Hartmann
tobias.hartmann at oracle.com
Wed Sep 3 11:10:24 UTC 2014
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
>>
More information about the hotspot-dev
mailing list