Review request: 8024751: Fix bugs in TraceMetadata

[Stefan Karlsson]

On 09/13/2013 09:48 AM, Bernd Eckenfels wrote:
> 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.

You are probably right, and I'm not going to defend the current 
placement of this trace code. But for this specific change I just want 
to fix the crash and the other misplaced print out.

>
> Is PTR_FORMAT right for word_size()

No, it should be SIZE_FORMAT. I'll change it.

> and would it be better to output bytes?

In my opinion, yes. However,  currently, most of print-outs guarded by 
TraceMetadata output numbers in words instead of bytes. Changing that 
will require another changeset.

> Is this also available as a event?

Not at the moment. But we plan to send this information out as an event. 
Maybe we can clean up some of the issues you have pointed when that is done.

thanks,
StefanK

>
> Bernd
>
> Am 13.09.2013 um 08:57 schrieb Stefan Karlsson 
> <stefan.karlsson at oracle.com <mailto: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/c3069bea/attachment.htm>