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

Tue Jun 3 18:42:27 UTC 2014

On 6/3/14 12:44 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> thanks for the review.
>
>> Much better. Please, use at least 'iter' name instead of 'it' which is confused with English word.
>> Also it may be better to call iterator NMethodIterator (with N).
>
> Done.
>
>> Use FLAG_SET_ERGO() (ergonomic) macro for SegmentedCodeCache setting.

I still see:

+     FLAG_SET_DEFAULT(SegmentedCodeCache, true);

Vladimir

>
> Done.
>
>> In CodeCache::allocate() you reversed your fix for 8029343: "CodeCache::allocate increments '_number_of_blobs' even if
>> allocation fails."
>
> That got lost during merging. Thanks!
>
> New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.01/
>
> Best,
> Tobias
>
>> thanks,
>> Vladimir
>>
>> On 6/2/14 4:06 AM, Tobias Hartmann wrote:
>>> Hi Vladimir,
>>>
>>> thanks for the feedback.
>>>
>>> On 17.04.2014 20:54, Vladimir Kozlov wrote:
>>>> The iterator look nice but I am not sure all our C++ compilers will
>>>> optimize out the allocation in end().
>>>>
>>>> I prefer our stile:
>>>>
>>>>    for (CodeCacheMethodIterator it(); !it.end(); it.next()) {
>>>>      nmethod* current = it.nmethod();
>>>
>>> I re-implemented the iterator (now called MethodIterator) to use the
>>> following style which is also used by RelocIterator:
>>>
>>> MethodIterator it;
>>> while(it.next()) {
>>>      // use it.method()
>>> }
>>>
>>> It is now used by the code cache implementation as well as by the
>>> sweeper and other client code.
>>>
>>>> sweeper.cpp:
>>>>
>>>> *_current is not clear statement, as above I prefer explicit
>>>> _current.nmethod(). And _current != CodeCacheMethodIterator::end()
>>>> would be !_current.end()
>>>>
>>>
>>> Done.
>>>
>>> Further, I added a flag "SegmentedCodeCache" to completely
>>> enable/disable the segmentation of the code cache. Per default it is
>>> only enabled with TieredCompilation and a ReservedCodeCacheSize >= 240
>>> MB. This setting makes sure we do not have any problems with small code
>>> cache sizes on embedded systems.
>>>
>>> New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.00/
>>>
>>> Thanks,
>>> Tobias
>>>
>>>> Implementation of is_full() looks fine.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 4/17/14 2:29 AM, Tobias Hartmann wrote:
>>>>> Vladimir, thanks for the review.
>>>>>
>>>>> Updated webrev: http://cr.openjdk.java.net/~anoll/8015774/webrev.06/
>>>>>
>>>>> On 04/14/2014 11:38 PM, Vladimir Kozlov wrote:
>>>>>> One thing is bothering me is codecache structure expose to outside
>>>>>> code (loop in compileBroker.cpp, nmethod.cpp and
>>>>>> jvmtiCodeBlobEvents.cpp).
>>>>>> It would be nice to hide it.
>>>>>
>>>>> I introduced a CodeCacheMethodIterator that iterates over all methods in
>>>>> the code cache. It hides the code cache internal structures and is now
>>>>> used in sweeper.cpp, nmethod.cpp and jvmtiCodeBlobEvents.cpp. In
>>>>> compileBroker.cpp a new version of CodeCache::is_full is used to
>>>>> determine if the code cache is full.
>>>>>
>>>>> I further added a new trace type CODEBLOBTYPE to tracetypes.xml to
>>>>> display the code heap that is full in the CodeCacheFull JFR event.
>>>>>
>>>>>> Before pushing it we need to test. SA, dtrace and other components are
>>>>>> affected by these changes. Ask sqe and performance group for help.
>>>>>
>>>>> I will do that next.
>>>>>
>>>>> Thanks,
>>>>> Tobias
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 4/14/14 6:05 AM, Tobias Hartmann wrote:
>>>>>>> Hi Volker,
>>>>>>>
>>>>>>> please see comments inline.
>>>>>>>
>>>>>>> On 04/14/2014 02:54 PM, Volker Simonis wrote:
>>>>>>>> Hi Tobias,
>>>>>>>>
>>>>>>>> is this change intended for jdk8 or 9?
>>>>>>>
>>>>>>> the change is intended for JDK 9. The webrev is based on the JDK 9
>>>>>>> repository (http://hg.openjdk.java.net/jdk9/hs-comp/hotspot)
>>>>>>> changeset 6280.
>>>>>>>
>>>>>>>>   In any case, I saw that your
>>>>>>>> webrev is based on the hsx repository which I thought should not be
>>>>>>>> used any more.
>>>>>>>
>>>>>>> Why do you think that it is based on hsx? The earlier versions 00 - 03
>>>>>>> actually were, because I already started developing the patch last
>>>>>>> year,
>>>>>>> but the newest one is based on the JDK 9 repository.
>>>>>>>
>>>>>>> As Roland noticed, the default code heap sizes for arm/ppc were wrong
>>>>>>> (because tiered compilation is supported). The updated webrev can be
>>>>>>> found at:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~anoll/8015774/webrev.05/
>>>>>>> <http://cr.openjdk.java.net/%7Eanoll/8015774/webrev.05/>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Tobias
>>>>>>>
>>>>>>>> Could you please rebase your patch on jdk9 and also
>>>>>>>> update the corresponding ppc64 files in the webrev (as far as I can
>>>>>>>> see, that's only src/cpu/ppc/vm/c2_globals_ppc.hpp). I'd like to test
>>>>>>>> this on Linux/PPC64 and AIX as well.
>>>>>>>>
>>>>>>>> Thank you and best regards,
>>>>>>>> Volker
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Apr 14, 2014 at 1:56 PM, Tobias Hartmann
>>>>>>>> <Tobias.Hartmann at oracle.com>  wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> please review the new version of the following patch that adds
>>>>>>>>> support for
>>>>>>>>> multiple code heaps to the code cache.
>>>>>>>>>
>>>>>>>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8015774
>>>>>>>>> Webrev:http://cr.openjdk.java.net/~anoll/8015774/webrev.04/
>>>>>>>>>
>>>>>>>>> Short description:
>>>>>>>>> This change implements support for multiple code heaps in the code
>>>>>>>>> cache.
>>>>>>>>> The interface of the code cache was changed accordingly and
>>>>>>>>> references from
>>>>>>>>> other components of the VM were adapted. This includes the indirect
>>>>>>>>> references from:
>>>>>>>>> - the Serviceability Agent: vmStructs and the Java code cache
>>>>>>>>> interface
>>>>>>>>> (sun.jvm.hotspot.code.CodeCache)
>>>>>>>>> - the dtrace ustack helper script (jhelper.d)
>>>>>>>>> - the pstack support library libjvm_db.c
>>>>>>>>>
>>>>>>>>> Currently the code cache contains the following three code heaps
>>>>>>>>> each of
>>>>>>>>> which contains CodeBlobs of a specific type:
>>>>>>>>> - Non-Profiled methods: nmethods that are not profiled and native
>>>>>>>>> methods
>>>>>>>>> - Profiled methods: nmethods that are profiled
>>>>>>>>> - Non-methods: Non-methods like buffers and adapters
>>>>>>>>>
>>>>>>>>> By default the non-method code heap uses 3 MB plus additional space
>>>>>>>>> for the
>>>>>>>>> compiler buffers that is dependent on the number of compiler
>>>>>>>>> threads (see
>>>>>>>>> CodeCache::initialize_heaps). The remaining code cache space is
>>>>>>>>> distributed
>>>>>>>>> equally among the non-profiled and the profiled code heaps.
>>>>>>>>>
>>>>>>>>> Tested:
>>>>>>>>> JPRT, SPECjvm2008, SPECjbb2005, SPECjbb2013, Octane + Nashorn
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Tobias
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -------- Original Message --------
>>>>>>>>> Subject: Re: RFR (L): 8015774: Add support for multiple code heaps
>>>>>>>>> Date: Mon, 21 Oct 2013 17:47:48 +0200
>>>>>>>>> From: Albert Noll<albert.noll at oracle.com>
>>>>>>>>> To: Azeem Jiva<azeem.jiva at oracle.com>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> That is fine. As we realized the last two weeks, there is more work
>>>>>>>>> to be
>>>>>>>>> done to make multiple code heaps work effectively.
>>>>>>>>>
>>>>>>>>> Albert
>>>>>>>>>
>>>>>>>>> Von meinem iPhone gesendet
>>>>>>>>>
>>>>>>>>> Am 21.10.2013 um 17:36 schrieb Azeem Jiva<azeem.jiva at oracle.com>:
>>>>>>>>>
>>>>>>>>> I still think we should hold off for JDK8u20
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Azeem Jiva
>>>>>>>>> @javawithjiva
>>>>>>>>>
>>>>>>>>> On Oct 21, 2013, at 8:33 AM, Albert Noll<albert.noll at oracle.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Von meinem iPhone gesendet
>>>>>>>>>
>>>>>>>>> Anfang der weitergeleiteten E‑Mail:
>>>>>>>>>
>>>>>>>>> Von: Vladimir Kozlov<vladimir.kozlov at oracle.com>
>>>>>>>>> Datum: 11. Oktober 2013 19:38:07 MESZ
>>>>>>>>> An:hotspot-compiler-dev at openjdk.java.net
>>>>>>>>> Betreff: Re: RFR (L): 8015774: Add support for multiple code heaps
>>>>>>>>>
>>>>>>>>> This looks acceptable.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>
>>>>>
>>>
>