RFR (S): 8136679: JFR Event for adaptive IHOP

Tom Benson tom.benson at oracle.com
Wed Nov 18 22:34:28 UTC 2015


Hi Thomas,
In g1IHOPControl.cpp, the call to report_basic_ihop_statistics at line 
198 in G1AdaptiveIHOPControl::send_event passes 
_allocation_rate_s.last() for the last_allocation_duration argument.  
Looking at your other IHOP change, I think it actually is a rate, rather 
than a duration, so perhaps not what you want here. ?

I also noticed an extra-trivial typo in trace.xml: "since last GC in 
bytes/seconds"should be /second.

Looks fine otherwise.
Tom

On 11/6/2015 1:15 PM, sangheon.kim wrote:
> Hi Thomas,
>
> Looks good, reviewed.
>
> Thanks,
> Sangheon
>
>
> On 11/06/2015 02:47 AM, Thomas Schatzl wrote:
>> Hi Sangheon,
>>
>>    thanks for the review.
>>
>> On Thu, 2015-11-05 at 16:17 -0800, sangheon.kim wrote:
>>> Hi Thomas,
>>>
>>> Overall looks good.
>>> I have pretty trivial comments.
>>>
>>> ============================
>>> http://cr.openjdk.java.net/~tschatzl/8136679/webrev/src/share/vm/gc/shared/gcTraceSend.cpp.frames.html 
>>>
>>>
>>> 273 size_t last_allocation_ize,
>>>
>>> Typo of last_allocation_size.
>> Thanks for catching this.
>>
>>> ============================
>>> http://cr.openjdk.java.net/~tschatzl/8136679/webrev/src/share/vm/gc/shared/gcTrace.hpp.frames.html 
>>>
>>>
>>> 263 void report_adaptive_ihop_statistics(size_t additional_buffer_size,
>>> 264 double predicted_allocation_rate,
>>> 265 double predicted_marking_length,
>>> 266 bool prediction_active);
>>>
>>> Wrong indentation.
>> Fixed. Due to late renaming of "dynamic" to "adaptive" for all
>> identifiers.
>>
>>> ============================
>>> http://cr.openjdk.java.net/~tschatzl/8136679/webrev/src/share/vm/gc/shared/gcTraceSend.cpp.frames.html 
>>>
>>>
>>> 281 evt.set_thresholdPercentage(threshold * 100.0 / target_occupancy);
>>>
>>> Q) Is always 'target_occupancy > 0' ?
>>    I think I fixed all your concerns in these new webrevs:
>>
>> http://cr.openjdk.java.net/~tschatzl/8136679/webrev.0_to_1/ (diff)
>> http://cr.openjdk.java.net/~tschatzl/8136679/webrev.1/ (full)
>>
>> Thanks,
>>    Thomas
>>
>>
>



More information about the hotspot-gc-dev mailing list