Review request: 8024751: Fix bugs in TraceMetadata

[Bernd Eckenfels]

Question: why is the trace block testing for is_humongus itself?

I would expect the allocation code to branch somewhere for this condition anyway - would it make more sense to put the trace there? Especially if you want to maintain (and output) additional details/statistics.

Is PTR_FORMAT right for word_size() and would it be better to output bytes? Is this also available as a event?

Bernd

Am 13.09.2013 um 08:57 schrieb Stefan Karlsson <stefan.karlsson at oracle.com>:

> On 09/13/2013 08:22 AM, Bengt Rutisson wrote:
>> 
>> Hi Stefan,
>> 
>> On 9/12/13 10:00 PM, Stefan Karlsson wrote:
>>> http://cr.openjdk.java.net/~stefank/8024751/webrev.00/ 
>>> 
>>> Small fixes two fix some issues when TraceMetadata* flags are turned on. 
>>> 
>>> - TraceMetadataHumongousAllocation crashes. 
>>> - TraceMetadataChunkAllocation prints the same block_freelist() multiple times. 
>> 
>> Looks good. I'm fine with pushing this as is, but I think I would have preferred that this code:
>> 
>> 
>> 2369   if (next != NULL) {
>> 2370     if (TraceMetadataHumongousAllocation &&
>> 2371         SpaceManager::is_humongous(next->word_size())) {
>> 2372       gclog_or_tty->print_cr("  new humongous chunk word size " PTR_FORMAT,
>> 2373                              next->word_size());
>> 2374     }
>> 2375   }
>> 
>> was more like:
>> 
>> 2370     if (TraceMetadataHumongousAllocation &&
>> 2371         next != NULL && SpaceManager::is_humongous(next->word_size())) {
>> 2372       gclog_or_tty->print_cr("  new humongous chunk word size " PTR_FORMAT,
>> 2373                              next->word_size());
>> 2374     }
>> 
>> To me it makes it clearer that this is only a tracing section.
> 
> Fair enough. I'll change it.
> 
> thanks,
> StefanK
>> 
>> Thanks,
>> Bengt
>>> 
>>> thanks, 
>>> StefanK 
>>> 
>>> 
>> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130913/13bc49b0/attachment.htm>