RFR(S): 8170672: Event-based tracing to support classloader instances

[Markus Gronlund]

Hi David,

Yes i agree with your observeration, I think it has some history around it.

I will create a traceStream.cpp file and move the larger implementations there (I will also incorporate Lois comments there).

Thanks for pointing this out.

Cheers
Markus

-----Original Message-----
From: David Holmes 
Sent: den 7 december 2016 10:58
To: Markus Gronlund; Coleen Phillimore
Cc: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR(S): 8170672: Event-based tracing to support classloader instances

Hi Markus,

On 7/12/2016 7:50 PM, Markus Gronlund wrote:
> Hi Coleen,
>
> Thanks for your comments - unfortunately i did submit the changeset before I saw your input, sorry about that.
>
> As for the reasons for ResourceMark's in traceStream.hpp, it is mainly for Symbol::as_C_string(), rather than for the actual stream (which is relly just tty) - hope this clarifies a bit.

traceStream.hpp seems to have some very large function definitions for a header file! It is also now including a .inline.hpp which breaks the general rules. Why isn't all that code in the cpp file?

Thanks,
David
-----

> Thanks again for having a look, I agree CLD is better than the oop's for this purpose.
>
> Markus
>
>
> -----Original Message-----
> From: Coleen Phillimore
> Sent: den 6 december 2016 23:29
> To: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8170672: Event-based tracing to support 
> classloader instances
>
>
> http://cr.openjdk.java.net/~mgronlun/8170672/webrev01/src/share/vm/tra
> ce/traceStream.hpp.udiff.html
>
> If _st is a stream that you've passed in like logStream, I don't think you want a ResourceMark because the ResourceMark belongs outside where
> you created the stream.   Otherwise, it could get nested resource mark
> assertions.
>
> This change looks good.  I'm glad you pass the CLD rather than the class_loader oop.
>
> thanks,
> Coleen
>
>
> On 12/5/16 6:33 PM, Markus Gronlund wrote:
>> Greetings,
>>
>>
>>
>> Kindly asking for reviews for the following changeset:
>>
>>
>>
>> Bug/Enh: https://bugs.openjdk.java.net/browse/JDK-8170672
>>
>> Webrev: http://cr.openjdk.java.net/~mgronlun/8170672/webrev01/
>>
>> (this work is covered by an FC exception)
>>
>>
>>
>> Summary:
>>
>> Event-based tracing previously had little information about class loaders; a class loader was essentially treated no different than from a regular class (type information only).
>>
>> With JDK9, supported has been added to java.lang.ClassLoader to associate a name with an individual class loader instance.
>>
>>
>>
>> This changeset will allow the name information of individual class loader instances to be provided by the event-based tracing framework.
>>
>>
>>
>> Aux info:
>>
>> Some folding of the numerous macros completed as well.
>>
>>
>>
>> Thanks in advance
>> Markus
>>
>>
>