On Wed, 20 May 2026 08:13:47 GMT, Thomas Stuefe <[email protected]> wrote:
>> Robert Toyonaga has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> renaming and fix uintx type
>
> src/hotspot/os/windows/os_windows.cpp line 499:
>
>> 497: }
>> 498:
>> 499: for (int i = 0; i < pages_to_query; i++) {
>
> signed vs unsigned? Did VC++ accept this?
Yes, good catch. The compiler didn't complain. Maybe it's able to deduce that
`pages_to_query` is capped at 1024. I've changed it now to `uintx` just incase.
> src/hotspot/os/windows/os_windows.cpp line 518:
>
>> 516: if (live_start != nullptr) {
>> 517: assert(live_pages > 0, "Must have live region");
>> 518: assert(live_pages <= size / page_sz, "Region cannot be smaller than
>> size of live pages");
>
> Maybe a bit clearer (same on POSIX)
> Suggestion:
>
> assert(live_pages <= size / page_sz, "resident size exceeds region
> size?");
I've adopted your suggestion
> src/hotspot/share/runtime/os.hpp line 447:
>
>> 445: // The search begins at the provided start address.
>> 446: // Returns true after the first contiguous live region is found,
>> otherwise false if none found.
>> 447: static bool live_in_range(address start, size_t size, address&
>> committed_start, size_t& committed_size);
>
> We probably want to rename the output parameters too.
Yes good point
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31124#discussion_r3276744838
PR Review Comment: https://git.openjdk.org/jdk/pull/31124#discussion_r3276747057
PR Review Comment: https://git.openjdk.org/jdk/pull/31124#discussion_r3276741146
