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

Coleen Phillimore coleen.phillimore at oracle.com
Wed Aug 12 17:00:58 UTC 2020



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


>
> Thanks,
> Lois



More information about the hotspot-dev mailing list