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

Thu Nov 6 11:01:52 UTC 2014

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/20141106/0ea403ba/attachment-0001.html>