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

Bengt Rutisson bengt.rutisson at oracle.com
Thu Nov 6 12:53:44 UTC 2014



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/20141106/c923df66/attachment.html>


More information about the hotspot-gc-dev mailing list