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

Wed Jun 4 06:04:27 UTC 2014

Hi Vladimir,

On 03.06.2014 20:42, Vladimir Kozlov wrote:
> 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);

Sorry, I accidentally reverted it.

New webrev: http://cr.openjdk.java.net/~thartmann/8015774/webrev.02/

Thanks,
Tobias

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