RFR: JDK-8055845 - Add trace event for promoted objects

Staffan Friberg staffan.friberg at oracle.com
Wed Nov 19 16:23:26 UTC 2014


Hi,

Erik and Tomas asked me to separate the G1 changes which was pushed 
separately in JDK-8064473.
Erik also had some further comments offline that I have taken care of.

Here is the new webrev, cr.openjdk.java.net/~sfriberg/8055845/webrev.06

Thanks,
Staffan


On 11/06/2014 01:53 PM, Bengt Rutisson wrote:
>
>
> On 2014-11-06 14:00, Staffan Friberg wrote:
>> Good catch, fixed.
>>
>>       // Too large; allocate the object individually.
>>       obj = sp->par_allocate(word_sz);
>>       if (obj != NULL) {
>> gc_tracer()->report_promotion_outside_plab_event(old, word_sz, age, 
>> false);
>>       }
>>
>> Please let me know if anyone wants a full new webrev with this.
>
>
> Looks good. Reviewed.
>
> Bengt
>
>>
>> Thanks,
>> Staffan
>>
>>
>> On 11/06/2014 12:01 PM, Bengt Rutisson wrote:
>>>
>>> Hi Staffan,
>>>
>>> On 2014-11-06 11:12, Staffan Friberg wrote:
>>>> Hi,
>>>>
>>>> After further off list discussion it was decided to keep the 
>>>> gc_tracer in par_promote as is.
>>>>
>>>> I have uploaded a new webrev, 
>>>> http://cr.openjdk.java.net/~sfriberg/8055845/webrev.05
>>>>
>>>> The main change here is a rewrite of the G1 code which is cleaner 
>>>> and also reuses the read age. By sending the markOop down through 
>>>> the call we can now trust the read age and do not need to reread it 
>>>> when incrementing which improves the YC performance slightly as it 
>>>> avoids the rather complex bit extraction.
>>>
>>>
>>> Looks good to me. One detail in parNewGeneration.cpp:
>>>
>>>  274     } else {
>>>  275       // Too large; allocate the object individually.
>>>  276 gc_tracer()->report_promotion_outside_plab_event(old, word_sz, 
>>> age, false);
>>>  277       obj = sp->par_allocate(word_sz);
>>>  278     }
>>>
>>> Seems like par_allocate() return NULL. Maybe we should check that 
>>> before reporting the event. Similarly to what you do in the other 
>>> GCs, for example G1:
>>>
>>> g1Allocator.cpp
>>>
>>>  141     obj = _g1h->par_allocate_during_gc(purpose, word_sz, context);
>>>  142     if (obj != NULL
>>>  143         && 
>>> _g1h->_gc_tracer_stw->should_report_promotion_outside_plab_event()) {
>>>  144       bool tenured = 
>>> _g1h->heap_region_containing_raw(obj)->is_old();
>>>  145 _g1h->_gc_tracer_stw->report_promotion_outside_plab_event(old, 
>>> word_sz,
>>>  146 age, tenured);
>>>  147     }
>>>
>>> Thanks,
>>> Bengt
>>>
>>>
>>>
>>>>
>>>> Cheers,
>>>> Staffan
>>>>
>>>> On 09/15/2014 02:06 PM, Bengt Rutisson wrote:
>>>>>
>>>>> Hi Staffan,
>>>>>
>>>>> psPromotionManager.inline.hpp
>>>>>
>>>>> I think the PSPromotionManager::copy_to_survivor_space() might 
>>>>> send multiple events. If the allocation to the young gen fails we 
>>>>> will fall through to do an old gen allocation. But we send the 
>>>>> events before we realize that the allocation has failed, i.e. 
>>>>> new_obj is NULL.
>>>>>
>>>>> I talked to Erik a bit about how to handle the gc_tracer in 
>>>>> par_promote. He'll get back to you with some thoughts on that.
>>>>>
>>>>> Thanks,
>>>>> Bengt
>>>>>
>>>>>
>>>>> On 2014-09-06 00:20, Staffan Friberg wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I have uploaded a new webrev here, 
>>>>>> cr.openjdk.java.net/~sfriberg/8055845/webrev.03
>>>>>>
>>>>>> It contains several changes
>>>>>>
>>>>>>     - Split event into two events (PromoteObjectInNewPLAB, 
>>>>>> PromoteObjectOutsidePLAB)
>>>>>>     - Moved events to "vm/gc/detailed/PromoteObject..."
>>>>>>     - Supporting ParNew+CMS and ParNew+SerialOld tenuring
>>>>>>          - Not sure if the way I do it with passing the 
>>>>>> ParNewTracer is the best solution, please let me know if you have 
>>>>>> an idea how to improve it
>>>>>>     - Simplified the G1 code to avoid sending age and having a 
>>>>>> single call site
>>>>>>     - Fixed so that the generated event has size information in 
>>>>>> bytes rather than words
>>>>>>
>>>>>> Thanks for offline comments and suggestions from Dmitry and Thomas.
>>>>>>
>>>>>> Cheers,
>>>>>> Staffan
>>>>>>
>>>>>> On 08/29/2014 03:32 PM, Staffan Friberg wrote:
>>>>>>> Hi Erik,
>>>>>>>
>>>>>>> On 08/28/2014 11:34 PM, Erik Helin wrote:
>>>>>>>> (it seems like we lost hotspot-gc-dev at openjdk.java.net 
>>>>>>>> somewhere in this thread, I'm adding it back)
>>>>>>>>
>>>>>>>> On 2014-08-28 23:15, Staffan Friberg wrote:
>>>>>>>>> Hi Erik,
>>>>>>>>>
>>>>>>>>> Thanks for the comments.
>>>>>>>>>> - Aren't the events for promotion to the tenured generation 
>>>>>>>>>> (SerialOld)
>>>>>>>>>>   and the CMS generation missing?
>>>>>>>>> The reason for leaving out these two, Serial completely and CMS
>>>>>>>>> promotion, was due to that neither as far as I understand make 
>>>>>>>>> use of
>>>>>>>>> PLABs.
>>>>>>>>
>>>>>>>> I might be wrong here, but looking at the function 
>>>>>>>> TenuredGeneration::par_promote (in tenuredGeneration.cpp) it 
>>>>>>>> looks to me like SerialOld is using PLABs when ParNew is 
>>>>>>>> promoting objects from young to old.
>>>>>>>>
>>>>>>>> As for CMS, looking at 
>>>>>>>> ConcurrentMarkSweepGeneration::par_promote (in 
>>>>>>>> concurrentMarkSweepGeneration.cpp) it seems like each 
>>>>>>>> CMSParGCThreadState has a CFLS_LAB (CompactibleFreeListSpace 
>>>>>>>> Local Allocation Buffer) that is a thread-local allocation 
>>>>>>>> buffer. See compactibleFreeListSpace.{hpp,cpp} for more details.
>>>>>>>>
>>>>>>>> Given this, I think we should add events for Serial and CMS as 
>>>>>>>> well.
>>>>>>>>
>>>>>>>
>>>>>>> Ok I see what you mean with CMS, basically the equivalent to 
>>>>>>> getting a PLAB would be when we refill the CFLS_LAB in the alloc 
>>>>>>> function. It still does the allocation to a small memory (~ size 
>>>>>>> of object) area from the freelist, but at least we did extra 
>>>>>>> work to refill the local CFLS_LAB. Need to do some analysis to 
>>>>>>> see how often we refill so the overhead doesn't get too high.
>>>>>>> The only issue I run into is how I can in a nice way get access 
>>>>>>> to the ParNewTracer from ParNewGeneration to call the report 
>>>>>>> function. Let's sync up next week and see how it can be solved.
>>>>>>>
>>>>>>> The tenured GC requires something similar to be supported, 
>>>>>>> however since ParNewGC is deprecated for usage without CMS in 
>>>>>>> JDK 8 we might want to skip that combination.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On 2014-08-28 23:15, Staffan Friberg wrote:
>>>>>>>>>> - Would it make sense to differentiate, either by separate 
>>>>>>>>>> events or by
>>>>>>>>>>   a field in a event, between promotions to to-space and to 
>>>>>>>>>> the old
>>>>>>>>>>   generation?
>>>>>>>>>> - The are two events for TLAB allocations,
>>>>>>>>>>   java/object_alloc_in_new_TLAB and 
>>>>>>>>>> java/object_alloc_outside_TLAB.
>>>>>>>>>>   What do you think about using two events for PLAB 
>>>>>>>>>> allocations as well:
>>>>>>>>>>   - java/object_alloc_in_new_PLAB
>>>>>>>>>>   - java/object_alloc_outside_PLAB
>>>>>>>>> I think this is a matter of taste and probably how similar we 
>>>>>>>>> want the
>>>>>>>>> event to be to the existing allocation event. I personally 
>>>>>>>>> prefer a
>>>>>>>>> single event but if the GC team and serviceability team feel 
>>>>>>>>> it would be
>>>>>>>>> better to have two I can certainly rewrite. The reason for me 
>>>>>>>>> preferring
>>>>>>>>> a single event is just ease of analysis, you can easily filter 
>>>>>>>>> a list of
>>>>>>>>> events on a field, it is harder to merge two different events 
>>>>>>>>> with
>>>>>>>>> different fields and work with them in an straight forward 
>>>>>>>>> fashion.
>>>>>>>>>
>>>>>>>>> Any general preference for having a single or multiple events?
>>>>>>>>
>>>>>>>> I would prefer to have two events in this case and try to 
>>>>>>>> follow the existing allocation events as much as possible (both 
>>>>>>>> in naming and in style). Keeping the tenured field (I missed it 
>>>>>>>> the first time I read the patch) is good.
>>>>>>>>
>>>>>>> Yes, tenured would be independent of having two events, only 
>>>>>>> PLAB size and directAllocation would be affected when having two 
>>>>>>> event types.
>>>>>>>
>>>>>>> *Erik Gahlin*, any preference from your end?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On 2014-08-28 23:15, Staffan Friberg wrote:
>>>>>>>>>> - In PSPromotionManager, instead of utilizing the C++ 
>>>>>>>>>> friendship with
>>>>>>>>>>   PSScavenge, should we add a getter function for the gc_tracer?
>>>>>>>>> Created a getter function.
>>>>>>>>
>>>>>>>> Thanks! If you make report_promotion_sample const, then the 
>>>>>>>> getter can return a const ParallelScavengeTracer*, right?
>>>>>>>>
>>>>>>> Done
>>>>>>>
>>>>>>> //Staffan
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20141119/f7e534df/attachment.html>


More information about the hotspot-gc-dev mailing list