RFR: 8038332: The trace event vm/class/load is not always being sent
Jiangli Zhou
jiangli.zhou at oracle.com
Wed Jun 22 17:53:34 UTC 2016
Hi Max,
I see there is already an ‘if (!HAS_PENDING_EXCEPTION)’ check at line 1170 before the debug_only code. Maybe you cloud place the post_class_load_event() in there so it only post the event when there is no pending exception. That way you don’t need to change the existing logic and add the additional checks.
Thanks,
Jiangli
> On Jun 20, 2016, at 12:08 PM, Max Ockner <max.ockner at oracle.com> wrote:
>
> David,
>
> New webrev: http://cr.openjdk.java.net/~mockner/8038332.02/src/share/vm/classfile/systemDictionary.cpp.cdiff.html
> I have added the check you suggested before triggering the event:
>
> if (HAS_PENDING_EXCEPTION || k.is_null()) {
> return NULL;
> }
>
> Thanks,
> Max
>
> On 6/13/2016 6:40 PM, David Holmes wrote:
>> Hi Max,
>>
>> On 14/06/2016 3:42 AM, Max Ockner wrote:
>>> Jiangli,
>>> Thanks for looking. I didn't see anything that looked like it might
>>> produce duplicate events. However, I did see some additional places
>>> where it looks like no event is fire.
>>>
>>> Can anyone point me to the event specs?
>>
>> I'm not sure there is any spec for this. Even JFR doesn't seem to document individual events and when they are triggered.
>>
>> Your change does not look right however as you are posting the classload event regardless of the exception state. If you look at SystemDictionary::resolve_instance_class_or_null, it only posts after checking the load was successful:
>>
>> 892 if (HAS_PENDING_EXCEPTION || k.is_null()) {
>> 893 return NULL;
>> 894 }
>> 895
>> 896 post_class_load_event(class_load_start_time, k, class_loader);
>>
>> Similarly, SystemDictionary::parse_stream, has various CHECK macros that will cause a return on exception, prior to getting to the point of posting the load event. So you also need to add a check for a pending exception and that k is not null, I think, before posting the event.
>>
> I have added this check.
>> BTW I would have expected to see trace-events generated at approximately the same locations as the corresponding JVMTI events. That does not seem to be the case which seems very strange to me. The notion of "loading a class" seems to be spread across far too many functions to me.
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Max
>>>
>>> On 6/9/2016 4:39 PM, Jiangli Zhou wrote:
>>>> Hi Max,
>>>>
>>>> Looks ok. The only possible issue is more than one event might be sent
>>>> in some of the call paths. But my quick search didn’t find any of such
>>>> case.
>>>>
>>>> Thanks,
>>>> Jiangli
>>>>
>>>>> On Jun 9, 2016, at 11:05 AM, Max Ockner <max.ockner at oracle.com> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Please review this small fix which causes the vm/class/load event to
>>>>> be fired from JVM_DefineClass() and JVM_DefineClassWithSource().
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~mockner/8038332/
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8038332
>>>>>
>>>>> The vm/class/load event (EventClassLoad) was previously fired in 2
>>>>> places:
>>>>>
>>>>> SystemDictionary::parse_stream
>>>>> SystemDictionary::resolve_instance_class_or_null
>>>>>
>>>>> parse_stream is the standard option for creating a klass from a
>>>>> stream, but JVM_DefineClass uses a different function:
>>>>>
>>>>> SystemDictionary::resolve_from_stream.
>>>>>
>>>>> This did not fire a vm/class/load event. Now it does fire the event.
>>>>>
>>>>> Sanity tested with jtreg runtime. Issue was reproduced and tested
>>>>> using the reproducer script attached to the bug
>>>>>
>>>>> Thanks,
>>>>> Max
>>>
>
More information about the hotspot-dev
mailing list