RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

[David Holmes]

On Wed, 16 Aug 2023 23:56:58 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Fix MaxElementPrintSize and casts. Also fixed miscellaneous -Wconversion warnings in runtime code.  This is the last one I'm going to do for runtime for a while.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change size of op_index back.

src/hotspot/share/logging/logOutput.cpp line 69:

> 67: 
> 68: static int tag_cmp(const LogTagType *a, const LogTagType *b) {
> 69:   return primitive_compare(a, b);

This looks very odd given we are dealing with pointers not primitives.

src/hotspot/share/memory/metaspace.cpp line 711:

> 709:     // it can get as low as 1:2. It is not a big deal though since ccs is only
> 710:     // reserved and will be committed on demand only.
> 711:     size_t max_ccs_size = (size_t)((double) MaxMetaspaceSize * 0.8);

Really no need for FP arithmetic here - we could just do:

size_t max_ccs_size =  8 * (MaxMetaspaceSize / 10);

src/hotspot/share/services/threadService.hpp line 107:

> 105:   static jlong get_peak_thread_count()        { return _peak_threads_count->get_value(); }
> 106:   static int get_live_thread_count()          { return _atomic_threads_count; }
> 107:   static int get_daemon_thread_count()        { return _atomic_daemon_threads_count; }

Given all the other jlong usage in these functions, and that these are used for the return value of  `jlong get_long_attribute(jmmLongAttribute att)` in management.cpp, I think these should stay as jlong returning functions.

src/hotspot/share/services/threadStackTracker.cpp line 46:

> 44: 
> 45: int ThreadStackTracker::compare_thread_stack_base(const SimpleThreadStackSite& s1, const SimpleThreadStackSite& s2) {
> 46:   return primitive_compare(s1.base(), s2.base());

Again these are not primitive values, but addresses, so this looks odd to me.

src/hotspot/share/utilities/elfFile.cpp line 792:

> 790:   // We must align to twice the address size.
> 791:   uint8_t alignment = DwarfFile::ADDRESS_SIZE * 2;
> 792:   uint64_t padding = alignment - (_reader.get_position() - _section_start_address) % alignment;

64-bit seems a waste here. I would just cast the result to uint8_t. The % operator ensures it is within range.

src/hotspot/share/utilities/elfFile.hpp line 486:

> 484:     DwarfFile* _dwarf_file;
> 485:     MarkedDwarfFileReader _reader;
> 486:     uintptr_t _section_start_address;

This seems suspicious - is this a 32-bit value or 64-bit?

src/hotspot/share/utilities/elfFile.hpp line 851:

> 849:       void reset_fields();
> 850:       // Defined in section 6.2.5.1 of the DWARF spec 4. add_to_address_register() must always be executed before set_index_register.
> 851:       void add_to_address_register(uint8_t operation_advance, const LineNumberProgramHeader& header);

Not clear how you decided/determined that `operation_advance` should always be 8-bit?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296554448
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296555979
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296558300
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296559830
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296569150
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296594924
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1296595284