[9] RFR (L): 8015774: Add support for multiple code heaps
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu Sep 4 18:49:45 UTC 2014
The test misses @run command.
Why you get ""TieredCompilation is disabled in this release."? Client VM?
What happens if we run with TieredStopAtLevel=1?
Thanks,
Vladimir
On 9/4/14 3:38 AM, Tobias Hartmann wrote:
> Thank you, Vladimir.
>
> I added a test that checks the result of segmented code cache related VM
> options.
>
> New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.06/
>
> Can I get a second review please?
>
> Best,
> Tobias
>
> On 04.09.2014 00:45, Vladimir Kozlov wrote:
>> 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
>>>>>
>>>
>
More information about the hotspot-dev
mailing list