[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR(XXS): Event Based tracing framework trace_id's to be reassigned for CDS klasses
From:       Markus Grönlund <markus.gronlund () oracle ! com>
Date:       2014-04-25 7:58:54
Message-ID: 1fb57186-1aca-4417-922f-1683b1c9c50e () default
[Download RAW message or body]

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@openjdk.java.net; serviceability-dev@openjdk.java.net \
                serviceability-dev@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@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@openjdk.java.net
> <mailto:serviceability-dev@openjdk.java.net>
> serviceability-dev@openjdk.java.net
> <mailto:serviceability-dev@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
> 


[prev in list] [next in list] [prev in thread] [next in thread]