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