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

Tue Jun 3 00:00:48 UTC 2014

Hi, Tobias

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

Use FLAG_SET_ERGO() (ergonomic) macro for SegmentedCodeCache setting.

In CodeCache::allocate() you reversed your fix for 8029343: 
"CodeCache::allocate increments '_number_of_blobs' even if allocation 
fails."

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