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/serviceability-dev/attachments/20141119/f7e534df/attachment-0001.html>
More information about the serviceability-dev
mailing list