RFR: Refactoring GC phase and heap allocation tracking out of policy

[Christine Flood]

This is good stuff.  Fine by me.

Thank You,

Christine


On Mon, Sep 25, 2017 at 11:15 AM, Zhengyu Gu <zgu at redhat.com> wrote:

>
>
> On 09/23/2017 06:50 AM, Aleksey Shipilev wrote:
>
>> On 09/23/2017 02:59 AM, Zhengyu Gu wrote:
>>
>>> Refactoring GC phase timing tracking and heap allocation tracking out of
>>> ShenandoahCollectorPolicy.
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/prof_refactor/web
>>> rev.00/index.html
>>>
>>
>> I like it!
>>
>> Would be interesting to try and conditionalize timing gathering on
>> gc+stats. It seems wasteful to
>> poll timers when we don't consume the data. Does not have to be in this
>> patch though, but maybe the
>> change should have that work in mind.
>>
>
> Our heuristics need timing feed also.
>
>
>> Few nits:
>>
>>   *) ShenandoahGCPhaseTimings -> ShenandoahPhaseTimings?
>>
>>   *) ShenandoahHeap::gc_phase_timings() -> ShenandoahHeap::phase_timings(
>> )?
>>
>>   *) ShenandoahGCPhaseTimings::TimingPhase ->
>> ShenandoahPhaseTimings::Phase?
>>
>> Okay.
>
>   *) Do we need the forward declaration in shenandoahCollectorPolicy.hpp?
>> outputStream was used
>> before in this class...
>>
>>    41 class outputStream;
>>
>
> Well, I think we should avoid indirect dependencies ...
>
>
>>   *) outputStream must have cr() methods for this?
>>
>>    1293     ls.print_cr("");
>>    1294     ls.print_cr("");
>>
>> Fixed.
>
> http://cr.openjdk.java.net/~zgu/shenandoah/prof_refactor/webrev.01/
>
> Reran all tests.
>
> Thanks,
>
> -Zhengyu
>
>
>
>> Thanks,
>> -Aleksey
>>
>>