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-...@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

Reply via email to