On Fri, 18 Aug 2023 00:24:07 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> If they stay jlong returns (note that these fields are in fact int), then we >> need to add casting to all the callers. Casting is worse than returning the >> correct types. If someone wants to make these fields jlong someday then >> they can propagate the change to the callers. This change corrects the >> types. > > I don't follow. The fields are int so cast them to jlong before returning > them. All the callers of these methods expect jlong so there can't be any > issue there. You don't need to cast from int to jlong. The callers of these methods expect int: src/hotspot/share/services/threadService.cpp:1103:55: warning: conversion from 'jlong' {aka 'long int'} to 'int' may change value [-Wconversion] 1103 | int init_size = ThreadService::get_live_thread_count(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ Changing int to jlong here, gets a new warning: src/hotspot/share/services/threadService.cpp:1104:54: warning: conversion from 'jlong' {aka 'long int'} to 'int' may change value [-Wconversion] 1104 | _threads_array = new GrowableArray<instanceHandle>(init_size); | ^~~~~~~~~ If you look up in the header file, the field is an int. It makes sense to return an int: static volatile int _atomic_threads_count; static volatile int _atomic_daemon_threads_count; Correcting the types means returning a type that matches the declaration so that its consistent. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1297894123