Request for review (XS): 7169056: Add gigabyte unit to proper_unit_for_byte_size() and byte_size_in_proper_unit()

[Bengt Rutisson]

Ulf,

You are correct. The LOG_G constant is wrong. If it should be based on 
the previous log constants it should be 3*LOG_K as you point out (it is 
not clear to me that this is more readable than just "30").

However, the LOG_G constant is not being used anywhere in the Hotspot 
code. I would prefer to remove it rather than fix it.

The LOG_K constant is never used directly. Just implicitly through 
LOG_M. And LOG_M is only used in one place: 
HeapRegionRemSet::setup_remset_size().

It seems strange to me to have this stuff in globalDefinitions.hpp. 
Unless anybody objects I'll file a CR to remove LOG_K and LOG_G and move 
LOG_M to heapRegionRemSet.cpp.

Thanks for catching this!
Bengt

On 2012-05-16 12:04, Ulf Zibis wrote:
> See:
>  164 const size_t LOG_K              = 10;
>  165 const size_t LOG_M              = 2 * LOG_K;
>  166 const size_t LOG_G              = 2 * LOG_M;
>
> Shouldn't it be
>  166 const size_t LOG_G              = 3 * LOG_K;
> ?
>
> -Ulf
>
>
> Am 15.05.2012 22:36, schrieb Bengt Rutisson:
>>
>> Hi all,
>>
>> I would like to push this simple change to allow for gigabyte units 
>> in some GC logging:
>> http://cr.openjdk.java.net/~brutisso/7169056/webrev.01/
>>
>> Jesper and John Cuthbertson have already looked at it, so in theory I 
>> am all set to push this. But I would like to give other people a 
>> chance to have a look at the change before I push it.
>>
>> Background
>> I attempted this change in a changeset that I pushed recently. It was 
>> not strictly related to those changes, so when I realized that I 
>> needed special treatment of 32 bit platforms I decided to push it as 
>> a separate change.
>>
>> Thanks,
>> Bengt
>>