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

Igor Veresov igor.veresov at oracle.com
Wed Jun 4 07:37:53 UTC 2014 

Looks good.

igor

On Jun 3, 2014, at 8:04 PM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:

> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140603/644fb572/attachment-0001.html>

More information about the hotspot-compiler-dev mailing list