[JFR: jdk8u-jfr-incubator] RFR: JDK-8238589: Necessary code cleanup in JFR for JDK8u

Aleksey Shipilev shade at redhat.com
Wed Feb 12 15:36:58 UTC 2020


On 2/12/20 1:42 PM, Mario Torre wrote:
>> *) In systemDictionary.cpp, this assert seems legit, no need to remove it? It guards from calling
>> the branch when jfr_event_handler_proxy is accidentally uninitialized:
>>
>>  1342       assert(jfr_event_handler_proxy != NULL, "invariant");
> 
> But if jfr_event_handler_proxy is null then also class_name needs to
> be null for the code to be executed, I don't think this is possible?

I don't want these two things to interact. So this is why you need the assert before the branch to
verify that we are testing against the sane value:

 #if INCLUDE_JFR
     assert(jfr_event_handler_proxy != NULL, "invariant");
     if (k.is_null() && (class_name == jfr_event_handler_proxy)) {
 ...

> http://cr.openjdk.java.net/~neugens/8238589/webrev.02/

Otherwise looks fine.

Stylistically, I don't think you need the comment at #endif of short blocks like:

 259 #if INCLUDE_JFR
 260   // If requested, return information on which thread held the bias
 261   if (biased_locker != NULL) {
 262     *biased_locker = biased_thread;
 263   }
 264 #endif // INCLUDE_JFR

...as it is dead obvious what that #endif belongs to.

-- 
Thanks,
-Aleksey



More information about the jdk8u-dev mailing list