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

Tue Jun 3 07:44:11 UTC 2014

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.

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