RFR: 8359227: Code cache/heap size options should be size_t

[Manuel Hässig]

On Fri, 13 Jun 2025 06:57:20 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Please review this change that makes the various code cache/heap size options
> consistently be of type size_t.
> 
> The shared declarations for these options were all uintx. These options all
> may have platform-defined values. Some of those platform-specific definitions
> were uintx, some were size_t, and some were intx(!). This change makes them
> all consistently size_t.
> 
> More details in the first comment.
> 
> Testing: mach5 tier1-6
> GHA testing in-progress

Thank you for working on this, @kimbarrett. Good to see more consistent types here.

The changes overall look good to me. I only have a question and some minor suggestions.

>One change in particular to note is the change in compilationPolicy.cpp. The
calculation of max_count, depending on explicit option values and such, could
potentially overflow its prior int type, effectively having a random value.
There is still a possibility of a nonsense result if ReservedCodeCacheSize and
CodeCacheMinimumUseSpace are poorly chosen. I'm leaving that pre-existing
issue to the compiler team to deal with.

Thank you for pointing this out. I'm currently looking at that part of the code in #25770 and will fix it there.

src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp line 61:

> 59: 
> 60: #include <type_traits>
> 61: 

Is this related to this change? I cannot find any usages from this header in the diff.

src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp line 550:

> 548: #define ADD_SIZE_T_FLAG(name) ADD_FLAG(size_t, name, BOXED_LONG)
> 549: #define ADD_INTX_FLAG(name)   ADD_FLAG(intx, name, BOXED_LONG)
> 550: #define ADD_UINTX_FLAG(name)  ADD_FLAG(uintx, name, BOXED_LONG)

Suggestion:

#define ADD_BOOL_FLAG(name)   ADD_FLAG(bool,   name, BOXED_BOOLEAN)
#define ADD_INT_FLAG(name)    ADD_FLAG(int,    name, BOXED_LONG)
#define ADD_SIZE_T_FLAG(name) ADD_FLAG(size_t, name, BOXED_LONG)
#define ADD_INTX_FLAG(name)   ADD_FLAG(intx,   name, BOXED_LONG)
#define ADD_UINTX_FLAG(name)  ADD_FLAG(uintx,  name, BOXED_LONG)

Feel free to ignore, but since you are already touching this, we might as well align it.

src/hotspot/share/runtime/arguments.cpp line 2451:

> 2449:         return JNI_EINVAL;
> 2450:       }
> 2451:       if (FLAG_SET_CMDLINE(ReservedCodeCacheSize, (size_t)long_ReservedCodeCacheSize) != JVMFlag::SUCCESS) {

Is this cast correct on a 32-bit platform, where `size_t` is not the same as `uint64_t`?

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

PR Review: https://git.openjdk.org/jdk/pull/25791#pullrequestreview-2924643009
PR Review Comment: https://git.openjdk.org/jdk/pull/25791#discussion_r2144994525
PR Review Comment: https://git.openjdk.org/jdk/pull/25791#discussion_r2144997094
PR Review Comment: https://git.openjdk.org/jdk/pull/25791#discussion_r2145007538