RFR: JDK-8055845 - Add trace event for promoted objects
Staffan Friberg
staffan.friberg at oracle.com
Tue Dec 2 18:33:01 UTC 2014
Hi,
After some further discussion we the GC team we have decided to split up
this patch further to make it easier to push.
Since this patch covers all GCs and multiple changes are on their way in
creating conflicts in the different GCs it is hard to push this in a
single commit.
So to make sure this moves forward and reduce contention we decided to
split it up in 4 pieces.
1. Event Implementation
2. Support for PS and ParOld
3. Support for CMS
4. Support for G1
I have created sub-tasks for each of these in the JDK-8055845.
I'm working on the first step and will send out a separate review for
that later today and the others will follow later.
Cheers,
Staffan
On 11/19/2014 08:23 AM, Staffan Friberg wrote:
> 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/20141202/f7f7348c/attachment-0001.html>
More information about the serviceability-dev
mailing list