Request for reviews (M): 7017124: VM statistic use 32-bit values which may overflow

[Vladimir Kozlov]

David Holmes wrote:
> Vladimir Kozlov said the following on 02/04/11 10:05:
>> Added new Atomic method inc_counter() to increment long values,
>> it is not precise on MP (lock is not used) but it is fine.
> 
> I strongly disagree! At least it has no place being in the Atomic:: set 
> of functions as it is not atomic. It is not even monotonic! It is 

It is atomic since it loads and stores all 64 bits of julong atomically.
Why we can call atomic methods Atomic::load() and Atomic::store()
which are used by new method but not this method?
And it is monotonic since value in memory only grows.

> extremely deceptive to do this. By all means change to julong where 
> needed, but unless they are truly going to be atomic updates (I 
> understand the overhead makes that prohibitive) then leave them as 
> simply  x+=i; statements.
> 
>> Fixed several output formats to use VM FORMAT macros.
>>
>> I have to remove v9 check assert from Atomic_move_long() since
>> it is called before VM_Version is initialized and C2 is built
>> for v9 only anyway.
>>
>> Removed unused Chunk::clean_chunk_pool().
> 
> Used by CI.

Restored.

>> I fixed only statistic which was interesting to me.
> 
> So the CR is misleading. It purports to deal with 32-bit stat overflows 
> but in fact only deals with some stats, and primarily you are using this 
> to define PrintMallocStat - so the CR should reflect that: "Fix some VM 
> stats to avoid 32-bit overflow"

OK.

> globals.hpp:
> 
> +           "print malloc/free statictics")       \
> 
> Typo: statictics

Fixed.

> src/share/vm/opto/phase.cpp
> 
> This change seems unrelated to the CR.

I removed it from these changes.

Thanks,
Vladimir

> 
> David
> -----