Review request: 8024751: Fix bugs in TraceMetadata

[Stefan Karlsson]

On 09/13/2013 10:09 AM, Stefan Karlsson wrote:
> 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.

On the other hand, maybe this is intentional. The other humongous 
allocation output also print with PTR_FORMAT.

Example:
Metadata humongous allocation:
   word_size 0x0000000000002620
   chunk_word_size 0x0000000000002628
     chunk overhead 0x0000000000000008
   new humongous chunk word size 0x0000000000002628

I'll leave this for now as well.

StefanK

>
>> 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/f3d4221e/attachment.htm>