Re: 回复： RFR: 8237499: JFR event for the location where a new thread starts
Erik Gahlin
erik.gahlin at oracle.com
Wed Jan 29 06:12:00 UTC 2020
Will do, but it could take a couple of weeks due other things
(conferences etc.).
Erik
On 2020-01-29 01:51, Denghui Dong wrote:
> Thanks! Please help me to push it if everything is ok.
>
> Denghui Dong
>
>
>
>
>
> 来自钉钉专属商务邮箱 <(null)>
>
> ------------------------------------------------------------------
> 发件人：Erik Gahlin<erik.gahlin at oracle.com>
> 日 期：2020年01月29日 05:50:57
> 收件人：Denghui Dong<denghui.ddh at alibaba-inc.com>;
> hotspot-jfr-dev<hotspot-jfr-dev at openjdk.java.net>
> 主 题：Re: RFR: 8237499: JFR event for the location where a new
> thread starts
>
> Looks good!
>
> Thanks for fixing this.
>
> Erik
>
> On 2020-01-28 18:07, Denghui Dong wrote:
>> Good explanation!
>> The webrev has been updated, please help review it :)
>>
>> Denghui Dong
>>
>> ------------------------------------------------------------------
>> From:Erik Gahlin <erik.gahlin at oracle.com>
>> Send Time:2020年1月28日(星期二) 20:38
>> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; hotspot-jfr-dev
>> <hotspot-jfr-dev at openjdk.java.net>
>> Subject:Re: RFR: 8237499: JFR event for the location where a
>> new thread starts
>>
>> Hi Denghui,
>>
>> On 2020-01-24 04:44, Denghui Dong wrote:
>> Hi Erik,
>> > set stackTrace="true" in metadata.xml
>> Actually, I thought about it before. But two thing let me
>> add a new field explicitly finally:
>> 1. EventThreadStart was also posted in jni.cpp, and there is
>> no need to record the stack trace, we cannot control it if
>> setting stackTrace="true".
>>
>> Not sure I understand, but in jni.cpp you could do what you
>> do today (set it to 0).
>>
>> For jfrThreadLocal.cpp, you could check
>> JfrEventSetting::has_stacktrace(EventThreadStart::eventId)).
>>
>>
>> 2. I think adding an explicit filed can make us and users
>> clear that the stack trace isn't recorded from the new thread
>> but the parent thread. And
>> there are also other events that have an explicit stack trace
>> field(e.g. ExecutionSample).
>>
>>
>> ExecutionSample and NativeMethodSample should probably have
>> stackTrace="true" as well.
>>
>> I don't think it matters from users point of view if the
>> field is added implicitly or explicitly. Maybe if it was
>> called "parentStackTrace", but that would lead to problems
>> for JMC and the parser since they depend on the field being
>> called "stackTrace". For us, it would make it more clear, but
>> perhaps not that important.
>>
>>
>> Another reason we may want to make the stack trace part of
>> the event is that it will make it easier to configure. For
>> example, we plan to add so you can set stack depth per event,
>> for example in the JMC recording wizard.
>> https://bugs.openjdk.java.net/browse/JDK-8134306
>>
>> Thanks
>> Erik
>>
>>
>> > change the label of the thread field from "Java Thread" to
>> "New Thread"
>> It makes sense, I will update it.
>>
>> Thanks
>> Denghui Dong
>> ------------------------------------------------------------------
>> From:Erik Gahlin <erik.gahlin at oracle.com>
>> Send Time:2020年1月23日(星期四) 21:44
>> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; hotspot-jfr-dev
>> <hotspot-jfr-dev at openjdk.java.net>
>> Subject:Re: RFR: 8237499: JFR event for the location where a
>> new thread starts
>>
>> Hi Denghui,
>>
>> Would it be possible to set stackTrace="true" in metadata.xml
>> instead of adding a new field explicitly?
>>
>> This allows users to turn on and off stack traces. You could
>> check if stack trace is enabled and only record if needed.
>>
>> I think it would be good to change the label of the thread
>> field from "Java Thread" to "New Thread".
>>
>> Thanks
>> Erik
>>
>> On 2020-01-19 09:13, Denghui Dong wrote:
>> Hi Erik
>> The webrev has been updated.
>> I reuse the event type "EventThreadStart" to report the
>> location where a new thread starts.
>> Add two fields "stackTrace" and "parentThread"
>> to EventThreadStart.
>> Please help review it, thanks.
>>
>> Best Regards
>> Denghui Dong
>> ------------------------------------------------------------------
>> From:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
>> Send Time:2020年1月18日(星期六) 21:25
>> To:Erik Gahlin <erik.gahlin at oracle.com>; hotspot-jfr-dev
>> <hotspot-jfr-dev at openjdk.java.net>
>> Subject:Re: RFR: 8237499: JFR event for the location where a
>> new thread starts
>>
>> Hi Erik,
>> Do you mean that we should reuse EventThreadStart rather
>> than a new event? It's a good idea, I will try it.
>>
>> Thanks
>> Denghui Dong
>> ------------------------------------------------------------------
>> From:Erik Gahlin <erik.gahlin at oracle.com>
>> Send Time:2020年1月18日(星期六) 21:12
>> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
>> Cc:hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net>
>> Subject:Re: RFR: 8237499: JFR event for the location where a
>> new thread starts
>>
>> Hi Denghui,
>>
>> This will be useful, but I wonder if we shouldn’t extend the Thread Start event with stack trace and the parent thread? It will require some additional logic, but it would be a cleaner solution.
>>
>> Thanks
>> Erik
>>
>> > On 18 Jan 2020, at 12:19, Denghui Dong
>> <denghui.ddh at alibaba-inc.com> wrote:
>> >
>> > Hi,
>> >
>> > Could I have a review of a change that introduces an event for the location where a new thread starts.
>> >
>> > Summary:
>> > There is already an event for thread start(EventThreadStart), but this event is committed in the new thread,
>> > so users can't know the location where the thread starts, hence, it's necessary to introduce another event,
>>
>> > it would be useful if the application exists thread leak problem.
>> >
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8237499
>> > Webrev: http://cr.openjdk.java.net/~ddong/8237499/
>> >
>> > Thanks
>> > Denghui Dong
>> >
>>
>
More information about the hotspot-jfr-dev
mailing list