RFR: 8185525: [Event Request] Add Tracing event for DictionarySizes

[coleen.phillimore at oracle.com]

On 4/9/19 11:44 AM, gerard ziemski wrote:
> Thank you Coleen for more feedback!
>
>
> On 4/9/19 9:43 AM, coleen.phillimore at oracle.com wrote:
>>>> http://cr.openjdk.java.net/~gziemski/8185525_rev2/src/hotspot/share/utilities/statistics.hpp.html 
>>>>
>>>>
>>>> Can you rename this file tableStatistics.cpp/hpp because 
>>>> "statistics" is too general and the class is called TableStatistics.
>>> I deliberately named the file "statistics.hpp", because I assume we 
>>> will be adding more JFR events in the future, and this file could 
>>> hold all the related code, which for now just comprises of table 
>>> statistics as you pointed out.
>>
>> Hi I don't agree with that.  I think if you want more different JFR 
>> statistics you could add files where they belong as 
>> differentStatistics.hpp
>
> Done.

Thanks!
>
>
>>>> http://cr.openjdk.java.net/~gziemski/8185525_rev2/src/hotspot/share/jfr/periodic/jfrPeriodic.cpp.udiff.html 
>>>>
>>>>
>>>> Is there anyway to parameterize these functions and/or add them to 
>>>> TableStatistics?
>>> I didn't want to add JFR dependency to TableStatistics. I'm unsure 
>>> what I can do more here, and whether it deserves the effort - 
>>> TableStatistics basically serves as a struct for passing event 
>>> attributes around, but I'm open to suggestions.
>>>
>>
>> I didn't think this should be moved from jfrPeriodic.cpp.  I thought 
>> it could be something like an X macro.
>>
>> Or just make this bit a function that they all call with event as 
>> parameter.
>>
>> + event.set_numberOfBuckets(statistics._number_of_buckets);
>> + event.set_numberOfEntries(statistics._number_of_entries);
>> + event.set_totalFootprint(statistics._total_footprint);
>> + event.set_maximumBucketCount(statistics._maximum_bucket_size);
>> + event.set_averageBucketCount(statistics._average_bucket_size);
>> + event.set_varianceOfBucketCount(statistics._variance_of_bucket_size);
>> + event.set_stdDevOfBucketCount(statistics._stddev_of_bucket_size);
>> + event.set_insertionRate(statistics._add_rate);
>> + event.set_removalRate(statistics._remove_rate);
>> + event.commit();
>
> Each of those JFR events are an instance of a different class, so the 
> best I can do is a macro here (otherwise I'd have to create a base 
> class for the TableStatistics events from which to extend our 6 table 
> events, but I'm not sure JFR architecture supports that - it generates 
> class automatically from the event's meta description)
>
> Updated webrev http://cr.openjdk.java.net/~gziemski/8185525_rev4/

Yes, that looks better to me.

+ //statistics.print(tty, "SymbolTable");


You should remove commented out code.   You can always add it back 
locally if you want to debug it again.  (I don't need to see this change).

thanks,
Coleen
>
>
> cheers