Re: [Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u

[guangyu.zhu]

ok, we will update the patch based on your comments.

Thanks,
Guangyu
------------------------------------------------------------------
Sender:Andrey Petushkov <andrey at azul.com>
Sent At:2019 Mar. 15 (Fri.) 21:44
Recipient:guangyu.zhu <guangyu.zhu at aliyun.com>
Cc:Mario Torre <neugens.limasoftware at gmail.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; denghui.ddh <denghui.ddh at antfin.com>
Subject:Re: [Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u

Hi Guangyu,

The G1 heap region type reporting looks good to me as well. Thanks a lot for doing it!

Regards,
Andrey

> On 14 Mar 2019, at 14:44, Andrey Petushkov <andrey at azul.com> wrote:
> 
> Hi Guangyu,
> 
> By the moment I've read first two patches (thread sampling and biased locks). These look good to me
> One very small note, I'd rather be verbose with the value of _trace_flag in thread.hpp. Just to prevent occasional use of the same value for another item of this enum in the (almost impossible) case that someone adds it
> 
> I need a bit more time to read through G1 heap region types one.
> BTW, it looks I've forgotten to mention that Azul is also missing G1 heap occupancy percent data which is indeed supported by Alibaba. Would you be able to integrate it also, please
> 
> Thanks a lot,
> Andrey
> 
>> On 14 Mar 2019, at 05:39, guangyu.zhu <guangyu.zhu at aliyun.com> wrote:
>> 
>> Ok. I look forward to the feedbacks from both of you.
>> 
>> Thanks,
>> Guangyu
>> ------------------------------------------------------------------
>> Sender:Mario Torre <neugens.limasoftware at gmail.com>
>> Sent At:2019 Mar. 13 (Wed.) 19:26
>> Recipient:Andrey Petushkov <andrey at azul.com>
>> Cc:guangyu.zhu <guangyu.zhu at aliyun.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; denghui.ddh <denghui.ddh at antfin.com>
>> Subject:Re: [Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u
>> 
>> Thanks!
>> 
>> I’m currently traveling but I’ll offer my review as soon as I get back next week.
>> 
>> Cheers,
>> Mario
>> On Tue 12. Mar 2019 at 09:35, Andrey Petushkov <andrey at azul.com> wrote:
>> Hi Guangyu,
>> 
>> Cool! Thank you so much! Will review changes ASAP
>> The backporting is still in progress. There are quite a lot of changes to consider so we'll likely finish some time after
>> April update release (mostly limited by testing resources capacity)
>> 
>> Regards,
>> Andrey
>> 
>>> On 12 Mar 2019, at 16:29, guangyu.zhu <guangyu.zhu at aliyun.com> wrote:
>>> 
>>> Hi Andrey, Mario
>>> 
>>> Is there any progress in backporting? We have completed the patch for the missing features. Please review.
>>> 
>>> - thread sampling:
>>> http://cr.openjdk.java.net/~luchsh/thread_sampling/
>>> 
>>> - biased locking events:
>>> http://cr.openjdk.java.net/~luchsh/hs_biasedlock/
>>> http://cr.openjdk.java.net/~luchsh/jdk_biasedlock/
>>> 
>>> - G1 heap region (heap summary is still missing, Alibaba's patch does not support heap summary either):
>>> http://cr.openjdk.java.net/~luchsh/g1region_type_change_event
>>> 
>>> Thanks,
>>> Guangyu
>>> 
>>> 
>>> ------------------------------------------------------------------
>>> Sender:Andrey Petushkov <andrey at azul.com>
>>> Sent At:2019 Mar. 6 (Wed.) 00:34
>>> Recipient:Mario Torre <neugens at redhat.com>
>>> Cc:guangyu.zhu <guangyu.zhu at aliyun.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; denghui.ddh <denghui.ddh at antfin.com>
>>> Subject:Re: [Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u
>>> 
>>> Hi Mario,
>>> 
>>>> On 4 Mar 2019, at 14:19, Mario Torre <neugens at redhat.com> wrote:
>>>> 
>>>> On Fri, Mar 1, 2019 at 8:09 PM Andrey Petushkov <andrey at azul.com> wrote:
>>>> 
>>>>>> I'm going through the patch right now, but yes, from what I see the
>>>>>> trace is removed. I had too a concern about this and was about to send
>>>>>> a note. I'm not quite sure what to do, because Trace has been removed
>>>>>> in 11 as far as I know, but removing mid stream in 8 may be a more
>>>>>> interesting issue, however this isn't a user facing API, it was always
>>>>>> meant to be internal to the JVM, so I don't quite know if there's
>>>>>> really a reason we shouldn't change it. This is one question for the
>>>>>> CSR group I think.
>>>>> Since trace was removed by the same commit as JFR was added to jdk11 my guess is that trace
>>>>> was used internally at Oracle to integrate closed implementation of JFR. With this sense I see no point
>>>>> to keep it. However if the guess is wrong and there some alternative implementation of trace event consumer
>>>>> I will be happy to return it back
>>>> 
>>>> Yes, I tend to agree with you, I do believe this is mostly an internal
>>>> API for easy of patching with the JFR code (which is almost
>>>> identical). The only concern is in the way the logging would be
>>>> triggered externally and the compile time options for it (I still see
>>>> a couple of instance where INCLUDE_TRACE is being used). As for
>>>> triggering the logs, I don't recall that 8 has any means of doing
>>>> this, I think some infrastructure came with 9 with the -Xlog option (I
>>>> didn't follow this however, I'm not sure the option ever landed in 9)?
>>>> In that case I guess it's safe to go after all.
>>> Right, the new logging infrastructure is badly missing here. Both Alibaba and Azul have added means of
>>> some JFR logging but far from what jdk11 could do. Let me check the rest of INCLUDE_TRACE places,
>>> IMHO we should get rid of all of them, but cannot tell for sure now
>>> 
>>> Andrey
>>>> 
>>>> Cheers,
>>>> Mario
>>>> --
>>>> Mario Torre
>>>> Associate Manager, Software Engineering
>>>> Red Hat GmbH <https://www.redhat.com>
>>>> 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
>>> 
>> 
>> 
>