RFR(XXS): Event Based tracing framework trace_id's to be reassigned for CDS klasses

[Markus Grönlund]

Thanks Erik, Coleen and Stefan.

/Markus

-----Original Message-----
From: Erik Helin 
Sent: den 25 april 2014 09:53
To: Markus Grönlund; Coleen Phillimore
Cc: hotspot-runtime-dev at openjdk.java.net; serviceability-dev at openjdk.java.net serviceability-dev at openjdk.java.net
Subject: Re: RFR(XXS): Event Based tracing framework trace_id's to be reassigned for CDS klasses

Hi Markus,

this looks good.

Thanks,
Erik

On 2014-04-24 19:06, Markus Grönlund wrote:
> Thanks Coleen,
>
> It's just a simple scalar assignment - it's ok to take yet another 
> incremented value if needed.
>
> /Markus
>
> *From:*Coleen Phillimore
> *Sent:* den 24 april 2014 18:56
> *To:* hotspot-runtime-dev at openjdk.java.net
> *Subject:* Re: RFR(XXS): Event Based tracing framework trace_id's to 
> be reassigned for CDS klasses
>
> On 4/24/14, 12:41 PM, Markus Grönlund wrote:
>
>     Thanks Stefan,
>
>     Yes, i think it's ok as long as the Klass  is not able to "do
>     anything" useful  - i.e. the Klass is not able to execute anything
>     which would involve its traceid.
>
>     So, I would assume the semantics of Klass::restore_unsharable_info()
>     would be similar in nature to a constructor? It prepares the Klass
>     for use.
>
> Stefan is right.   It's sort of a restartable constructor.   We keep the
> values "restored" so far if you get OOM while restoring the values.  
> You could conditionally if DumpSharedSpaces not initalize this field 
> in the Klass constructor and check if it's zero before calling 
> TRACE_INIT_ID, if TRACE_INIT_ID has side effects you only want once.  
> I was thinking earlier that it stores some sort of scalar to this 
> field and that would be ok to do more than once.
>
> As long as the Klass is coming out "prepared" with a unique ID 
> assigned, this will be fine.
>
>
> So in the rare case of OOM during restore_unshareable_info, you might 
> get an extra unique value if the class is successfully loaded again 
> (which is also rare except in our testing apparently).
>
> I think this fix is good.
>
> Coleen
>
>
> Thanks
>
> Markus
>
> *From:*Stefan Karlsson
> *Sent:* den 24 april 2014 18:30
> *To:* Markus Grönlund; serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>
> serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>; hotspot-runtime-dev
> *Subject:* Re: RFR(XXS): Event Based tracing framework trace_id's to 
> be reassigned for CDS klasses
>
> Hi Markus,
>
> On 2014-04-24 17:42, Markus Grönlund wrote:
>
>     Greetings,
>
>     Kindly asking for reviews for the following very small fix:
>
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8041723
>
>     Webrev: http://cr.openjdk.java.net/~mgronlun/8041723/webrev01/
>     <http://cr.openjdk.java.net/%7Emgronlun/8041723/webrev01/>
>
>
> Klass::restore_unshareable_info() might be called multiple times for a 
> given Klass. This can happen if OutOfMemoryErrors is thrown when the 
> Klass is loaded, and we later retry to load the Klass. Is it OK to 
> call
> TRACE_INIT_ID(this) multiple times for the same Klass?
>
> thanks,
> StefanK
>
>
> Description:
>
> The Event Based tracing framework assigns a unique traceid to Klass:es 
> for tracking purposes.
> Normally, a new Klass is assigned it's traceid inside the Klass 
> constructor.
> For Klass:es coming into the system via the ClassDataSharing (CDS) 
> mechanism, the old traceid for the Klass will be stale, hence a "new"
> traceid needs to be (re)assigned to the Klass.
>
> Thank you
>
> Markus
>