RFR (S) 8235765: Use of the long type should be avoided in shared code

[Lois Foltan]

On 8/12/2020 1:00 PM, Coleen Phillimore wrote:
>
>
> On 8/12/20 12:19 PM, Lois Foltan wrote:
>> On 8/12/2020 11:21 AM, Coleen Phillimore wrote:
>>> Summary: Changed some long declarations to uint64_t/int64_t or 
>>> unsigned int, depending on context.
>>>
>>> There are still 'long' declarations left in the code, but they 
>>> should be changed by developers when working in that code and not as 
>>> a blanket change.  I didn't change a couple of longs in 
>>> jfr/leakprofiler, for example.  These are the ones I changed though 
>>> with some explanation of why:
>>>
>>> src/hotspot/share/memory/filemap.hpp
>>>
>>> This can be negative so changed to int64_t.
>>>
>>> src/hotspot/share/runtime/mutex.cpp
>>>
>>> The PlatformMutex code takes jlong, which is signed, so that's why I 
>>> changed these to int64_t.
>>>
>>> runtime/interfaceSupport.inline.hpp
>>>
>>> These counters are actually intervals so I changed them to unsigned 
>>> int.
>>>
>>> src/hotspot/share/compiler/compileBroker.hpp
>>>
>>> _peak_compilation_time is signed because it is compared with jlong 
>>> which is signed.
>>> Same with total_compilation_time - elapsedTimer.milliseconds() 
>>> returns jlong.
>>>
>>> Tested with tier1-3.  Tier1 on linux-x64-debug, windows-x64-debug, 
>>> macosx-x64-debug, linux-aarch64-debug. Also built on: 
>>> linux-arm32,linux-ppc64le-debug,linux-s390x-debug,linux-x64-zero.
>>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2020/8235765.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8235765
>>>
>>> Thanks,
>>> Coleen
>>
>> Hi Coleen,
>>
>> Looks good.
>>
>> - runtime/sweeper.hpp
>>   This is the only file that I wondered why you changed long to 
>> int64_t for _total_nof_methods_reclaimed and 
>> _total_nof_c2_methods_reclaimed.  Note that the method 
>> NMethodSweeper::total_nof_methods_reclaimed returns an int. Could 
>> both of these fields be changed to int instead?
>
> Hi Lois, Thank you for looking at this.  Unfortunately, this was an 
> outdated webrev, can you hit reload?  I changed these fields to be 
> uint64_t because they're never signed.  It's likely that the number of 
> methods is never greater than an int, but since it was long to begin 
> with, I kept 64 bit until someone decides an 'int' is better. Since 
> number_of_codecache_sweeps is uint64_t, which seems like a lot too, 
> there could be that many nmethods reclaimed.  I retested with windows 
> just now to be sure.
>
> Thanks,
> Coleen

Yes, that looks better.  Thanks!
Lois

>
>
>>
>> Thanks,
>> Lois
>