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

[Lois Foltan]

On 3/5/2020 11:46 AM, Markus Gronlund wrote:
>
> 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).
>

Okay, thanks for the clarification.  I'm good with your change!
Lois

> 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 <markus.gronlund at oracle.com>
>     <mailto:markus.gronlund at oracle.com>;
>     hotspot-jfr-dev at openjdk.java.net
>     <mailto:hotspot-jfr-dev at openjdk.java.net>; Harold Seigel
>     <harold.seigel at oracle.com> <mailto:harold.seigel at oracle.com>; Lois
>     Foltan <lois.foltan at oracle.com> <mailto: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
>