On Fri, 18 Aug 2023 01:24:20 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> 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. I'd missed the GrowableArray code - the other callers expect jlong/64-bit. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1297911413