RFR(S): 8239376: JFR: assert(!cld->is_unsafe_anonymous()) failed: invariant

[Markus Gronlund]

Hi Lois,

 

Thanks for taking a look.

 

Actually, the package id == 0 for a typeArray_klass is not a change in behavior – it was in the previous version of write_klass() (although a bit obscure).

 

Thanks again

Markus

 

 

 

From: Lois Foltan 
Sent: den 5 mars 2020 17:33
To: Markus Gronlund <markus.gronlund at oracle.com>; Coleen Phillimore <coleen.phillimore at oracle.com>; hotspot-jfr-dev at openjdk.java.net; Harold Seigel <harold.seigel at oracle.com>
Subject: Re: RFR(S): 8239376: JFR: assert(!cld->is_unsafe_anonymous()) failed: invariant

 

On 3/5/2020 9:58 AM, Markus Gronlund wrote:



Hi Coleen,

 

Thank you! That made things a lot simpler:

 

Updated webrev: http://cr.openjdk.java.net/~mgronlun/8239376/webrev02/


Hi Markus,

I think this looks good.   write_klass will now return a package_id of 0 for a typeArray klass which I think is fine, just wanted to make sure I understood that change in behavior correctly?

Thanks,
Lois




 

Thanks again

Markus

 

From: Coleen Phillimore 
Sent: den 5 mars 2020 15:27
To: Markus Gronlund HYPERLINK "mailto:markus.gronlund at oracle.com"<markus.gronlund at oracle.com>; HYPERLINK "mailto:hotspot-jfr-dev at openjdk.java.net"hotspot-jfr-dev at openjdk.java.net; Harold Seigel HYPERLINK "mailto:harold.seigel at oracle.com"<harold.seigel at oracle.com>; Lois Foltan HYPERLINK "mailto:lois.foltan at oracle.com"<lois.foltan at oracle.com>
Subject: Re: RFR(S): 8239376: JFR: assert(!cld->is_unsafe_anonymous()) failed: invariant

 


You don't need this code to get package() because it's a virtual function in Klass, and objArrayKlass will return bottom_type().





+  klass = contextual_representative(klass);
+  if (klass->is_typeArray_klass()) {
+    return 0;
+  }
+  assert(klass->is_instance_klass(), "invariant");

Or here either;





 397     if (klass->is_typeArray_klass()) {
 398       return NULL;
 399     }
 400     assert(klass->is_instance_klass(), "invariant");
 401     return InstanceKlass::cast(klass)->package();


which gets rid of the InstanceKlass::cast() also.

The name contextual_representative() is really strange to read.  I'd just add the code inline to get_cld, to get bottom_klass().





172 static ClassLoaderData* get_cld(const Klass* klass) {
 173   assert(klass != NULL, "invariant");
  174   if (klass->is_objArrayKlass()) {
          klass = objArrayKlass::cast(klass)->bottom_klass();
      }
175   return is_unsafe_anonymous(klass) ?
 176     InstanceKlass::cast(klass)->unsafe_anonymous_host()->class_loader_data() : klass->class_loader_data();
 177 }


Coleen

On 3/5/20 8:08 AM, Markus Gronlund wrote:

Greetings,
 
Please review the following change set:
 
Bug: https://bugs.openjdk.java.net/browse/JDK-8239376 
Webrev: http://cr.openjdk.java.net/~mgronlun/8239376/webrev01/
Testing: jdk_jfr
Description: please see bug
 
Thanks
Markus